Skip to content
This repository has been archived by the owner on Dec 11, 2019. It is now read-only.

not prompted to restart Brave when changing preferences marked with an asterisk #10410

Closed
LaurenWags opened this issue Aug 10, 2017 · 2 comments

Comments

@LaurenWags
Copy link
Member

  • Did you search for similar issues before submitting this one?
    Yes

  • Describe the issue you encountered:
    Preferences marked with an asterisk are not prompting for Brave to be restarted. This was working in 0.18.20.

  • Platform (Win7, 8, 10? macOS? Linux distro?):
    macOS

  • Brave Version (revision SHA):
    Brave | 0.18.21
    rev | 263b6d5
    Muon | 4.3.9

  • Steps to reproduce:

    1. In Preferences change language (or any other preference marked with an asterisk)
    2. You are not prompted for restart.
  • Actual result:
    You are not prompted to restart Brave.

  • Expected result:
    Should be prompted to restart Brave.

  • Will the steps above reproduce in a fresh profile? If not what other info can be added?
    Yes

  • Is this an issue in the currently released version?
    No (was last working in 0.18.20)

  • Can this issue be consistently reproduced?
    Yes

  • Extra QA steps:
    1.
    2.
    3.

  • Screenshot if needed:

Actual:
language_change

Expected:
screen shot 2017-08-10 at 3 01 03 pm

  • Any related issues:
@LaurenWags LaurenWags added this to the 0.18.x Hotfix milestone Aug 10, 2017
@cndouglas
Copy link

Same here, with Brave 0.18.21 (263b6d5) on macOS 10.12.x.

@bbondy
Copy link
Member

bbondy commented Aug 10, 2017

getOrigin('about:preferences') would give about:
new:
getOrigin('about:preferences') gives null

And the issue manifests itself inside about:preferences changes when you get a restart notification

  const activeOrigin = getOrigin(this.props.activeFrame.get('location'))
    if (!activeOrigin) {
      return null
    }
...

returns null there

bbondy added a commit that referenced this issue Aug 10, 2017
Fix #10410

Auditors: @diracdeltas

Notes this is in the branch of code which requires muon only so it is already in unit tests but we test it with the wrong url parsing from node.
bbondy added a commit that referenced this issue Aug 10, 2017
Fix #10410

Auditors: @diracdeltas

Notes this is in the branch of code which requires muon only so it is already in unit tests but we test
dfperry5 pushed a commit to dfperry5/browser-laptop that referenced this issue Aug 18, 2017
Fix brave#10410

Auditors: @diracdeltas

Notes this is in the branch of code which requires muon only so it is already in unit tests but we test it with the wrong url parsing from node.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.