Closed Bug 1379269 Opened 2 years ago Closed 2 years ago

Toggling "Show accent color on title bar" in Windows settings is not picked up until restart

Categories

(Core :: Widget: Win32, defect, P1)

defect

Tracking

()

VERIFIED FIXED
mozilla56
Iteration:
57.3 - Sep 19
Tracking Status
firefox56 --- fixed
firefox57 --- verified

People

(Reporter: johannh, Assigned: jwatt)

References

Details

(Whiteboard: [reserve-photon-visual])

Attachments

(2 files)

When I check the "Show color in title bar" checkbox in Windows accent color settings I would expect Firefox to instantly change to the Windows accent color (like the settings screen does). It doesn't.
Jonathan, can we enable this somehow?
Component: Theme → Widget: Win32
Flags: needinfo?(jwatt)
Product: Firefox → Core
Attached patch patchSplinter Review
The '-moz-win-accentcolor' keyword is already live to changes to the color that is chosen for the Windows 10 accent color. This patch is required to invalidate the cached result of the '-moz-windows-accent-color-applies' media query when the Windows 10 accent color is enabled/disabled in the Windows settings.
Assignee: nobody → jwatt
Status: NEW → ASSIGNED
Flags: needinfo?(jwatt)
Attachment #8884696 - Flags: review?(dholbert)
Flags: qe-verify?
Priority: -- → P1
Whiteboard: [photon-visual][triage] → [photon-visual]
Pushed by dgottwald@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/33f7f73ffb36
Make the '-moz-windows-accent-color-applies' media query live to Windows 10 settings changes. r=dholbert
https://hg.mozilla.org/mozilla-central/rev/33f7f73ffb36
Status: ASSIGNED → RESOLVED
Closed: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla56
Iteration: --- → 56.3 - Jul 24
Flags: qe-verify? → qe-verify+
QA Contact: brindusa.tot
This doesn't seem to be working for me. In a current (7/20) Nightly, when I enable the Win10 option to show the titlebar accent color, I still have to restart Firefox for it to take effect (otherwise it continues to just show the default Firefox theme). Ditto for disabling the Windows setting (Firefox continues to use the accent color until it is restarted).

The color itself will update w/o a restart when I change it from the Windows dialog.
Status: RESOLVED → REOPENED
Flags: needinfo?(dholbert)
Resolution: FIXED → ---
[I think you meant to needinfo the patch-author, jwatt, who conveniently is back as of today according to his Bugzilla nick.  Transferring ni.]
Flags: needinfo?(dholbert) → needinfo?(jwatt)
After using this on Windows for a few days it seems like certain actions e.g. minimizing the browser window or opening a new window will cause the setting to apply correctly. We probably need to do out whatever these actions are doing to reset/invalidate the accent color.
> need to do out

Sigh, this was originally "find out" but I'm trying to say "we need to do whatever these actions are doing".
Strange. This works for certain windows such as the Browser Console. I'm pretty sure I was just using regular windows when I tested this though.
Flags: needinfo?(jwatt)
(In reply to Johann Hofmann [:johannh] from comment #9)
> After using this on Windows for a few days it seems like certain actions
> e.g. minimizing the browser window or opening a new window will cause the
> setting to apply correctly.

When resizing the window etc. causes the visual appearance to correctly refresh we end up getting the media query value under the stack:

  GetSystemMetric
  nsMediaQueryResultCacheKey::Matches
  nsCSSRuleProcessor::RefreshRuleCascade
  nsCSSRuleProcessor::MediumFeaturesChanged
  nsStyleSet::MediumFeaturesChanged
  mozilla::StyleSetHandle::Ptr::MediumFeaturesChanged
  nsPresContext::MediaFeatureValuesChanged
  nsPresContext::FlushPendingMediaFeatureValuesChanged
  mozilla::PresShell::DoFlushPendingNotifications
  nsIPresShell::FlushPendingNotifications
  mozilla::PresShell::DoFlushPendingNotifications
  nsIPresShell::FlushPendingNotifications
  nsDocument::FlushPendingNotifications
  mozilla::dom::BoxObject::GetPresShell
  mozilla::dom::BoxObject::GetFrame
  mozilla::dom::BoxObject::GetOffsetRect
  mozilla::dom::BoxObject::GetWidth
  mozilla::dom::BoxObject::Width
  mozilla::dom::BoxObjectBinding::get_width

It looks like the patch I needed really should be calling nsPresContext::MediaFeatureValuesChanged too.
Comment on attachment 8890350 [details] [diff] [review]
part 2 - Invalidate and notify when clearing cached media features for Windows accent c olor changes

Review of attachment 8890350 [details] [diff] [review]:
-----------------------------------------------------------------

r=me
Attachment #8890350 - Flags: review?(dholbert) → review+
Pushed by jwatt@jwatt.org:
https://hg.mozilla.org/integration/mozilla-inbound/rev/6db4b0af686c
part 2 - Invalidate and notify when clearing cached media features for Windows accent color changes. r=dholbert
Iteration: 56.3 - Jul 24 → ---
https://hg.mozilla.org/mozilla-central/rev/6db4b0af686c
Status: REOPENED → RESOLVED
Closed: 2 years ago2 years ago
Resolution: --- → FIXED
Iteration: --- → 57.1 - Aug 15
QA Contact: brindusa.tot → ovidiu.boca
I tested this on Windows 10 with FF Nightly 57.0a1(2017-08-16) and I still can reproduce this issue.
Status: RESOLVED → REOPENED
Flags: needinfo?(jwatt)
Resolution: FIXED → ---
Iteration: 57.1 - Aug 15 → ---
Whiteboard: [photon-visual] → [reserve-photon-visual]
As best I can tell we have some sort of race when there are multiple top level windows. Sometimes it works, sometimes it doesn't. Annoyingly it usually works when Firefox is run from a debugger with breakpoints set.

Ovidiu, did you notice any particular pattern of interaction that would cause it to work or not work?
Flags: needinfo?(jwatt) → needinfo?(ovidiu.boca)
Hi Jonathan,

We retested this and we found out that on private windows this issue is always reproducible, maybe this can help you. Also, we have a screen recorder with this issue, please go to this link: https://streamable.com/xrjfr
Flags: needinfo?(ovidiu.boca)
Verified this on latest Nightly 57.0a1, Build ID 20170914220209 with normal and private window and title bar gets the accent color.
Ovidiu, considering Brindusa has this fixed on her machine, can you confirm that it's not reproducible for you anymore?
Status: REOPENED → RESOLVED
Closed: 2 years ago2 years ago
Flags: needinfo?(ovidiu.boca)
Resolution: --- → FIXED
Hi Johann, I can't reproduce this anymore, if we encounter this issue in the future we will reopen it. Thanks
Status: RESOLVED → VERIFIED
Flags: qe-verify+
Flags: needinfo?(ovidiu.boca)
Iteration: --- → 57.3 - Sep 19
You need to log in before you can comment on or make changes to this bug.