1420166 - regression: context menu on about:config has lost its style

Status: VERIFIED FIXED

(Toolkit :: UI Widgets, defect)

Description

[Darkspirit]

Attachment: Screenshot_20171123_155113.png

Nightly 59 x64 20171123100420 de_DE b6bed1b710c3e22cab49f22f1b5f44d80286bcb9 @ Debian Testing (KDE, Radeon RX480)
fresh profile

1. open about:config
2. click the blue "I accept the risk!" button
3. do a rightclick

See attached screenshot.

mozregression --good 2017-11-22 --bad 2017-11-23
> 3:35.81 INFO: Last good revision: 87368e9f8bada46d65cd3d78984ad14e152a5e60
> 3:35.81 INFO: First bad revision: 5f57907f645bf30a72e0dd0a7013054fe328a3ce
> 3:35.81 INFO: Pushlog:
> https://hg.mozilla.org/integration/autoland/pushloghtml?fromchange=87368e9f8bada46d65cd3d78984ad14e152a5e60&tochange=5f57907f645bf30a72e0dd0a7013054fe328a3ce

> 5f57907f645b	Brian Grinstead — Bug 1416493 - Remove the menuseparator XBL binding;r=Gijs

Comment 1

[:Gijs (he/him)]

Does this also happen on about:preferences with e.g. context menus in the textboxes?

Comment 2

[Brian Grinstead [:bgrins]]

I guess it's not loading global.css: https://dxr.mozilla.org/mozilla-central/source/toolkit/components/viewconfig/content/config.xul. Is there a reason to not load global.css for about:config?

Based on the comment at the top of global.css (https://dxr.mozilla.org/mozilla-central/source/toolkit/themes/linux/global/global.css#5-7) I assumed it would be loaded in all XUL documents:

/* ===== global.css =====================================================
  == Styles that apply everywhere.
  ======================================================================= */

Comment 3

[Brian Grinstead [:bgrins]]

FWIW it looks like this is the only xul doc loading `chrome://global/skin/in-content/info-pages.css`: https://dxr.mozilla.org/mozilla-central/search?q=chrome%3A%2F%2Fglobal%2Fskin%2Fin-content%2Finfo-pages.css&redirect=true

Comment 4

[Darkspirit]

(In reply to :Gijs from comment #1)
> Does this also happen on about:preferences with e.g. context menus in the textboxes?

No, it seems to be fine there. (about:preferences#general > Settings search, Home page, file types)

Comment 5

[Brian Grinstead [:bgrins]]

Attachment: Bug 1420166 - Include global.css on about:config;

This fixes missing menu styling caused by Bug 1416493

Review commit: https://reviewboard.mozilla.org/r/202534/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/202534/

Comment 6

[Brian Grinstead [:bgrins]]

Pushed up a change with a reference to global.css.  This does make me wonder if we should be loading that file automatically in xul documents similar to xul.css (and maybe there's also a perf win to be had: https://dxr.mozilla.org/mozilla-central/source/layout/style/nsLayoutStylesheetCache.cpp#355-359)

Comment 7

[Brian Grinstead [:bgrins]]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=43bc62ed5e76001db512dca39899dad8d5769f66

Comment 8

[:Gijs (he/him)]

(In reply to Brian Grinstead [:bgrins] from comment #6)
> Pushed up a change with a reference to global.css.  This does make me wonder
> if we should be loading that file automatically in xul documents similar to
> xul.css (and maybe there's also a perf win to be had:
> https://dxr.mozilla.org/mozilla-central/source/layout/style/
> nsLayoutStylesheetCache.cpp#355-359)

Wanna file an exploratory perf bug to look into that? :)

Comment 9

[Brian Grinstead [:bgrins]]

(In reply to :Gijs from comment #8)
> Wanna file an exploratory perf bug to look into that? :)

Sure, setting needinfo to remind myself to do that next week

Comment 10

[:Gijs (he/him)]

Comment on attachment 8931400 [details]
Bug 1420166 - Include global.css on about:config;

https://reviewboard.mozilla.org/r/202534/#review207912

Nice and simple, rs=me

Comment 11

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/be5e92dc51ec
Include global.css on about:config;r=Gijs

Comment 12

[Tiberius Oros[:tiberius_oros]]

https://hg.mozilla.org/mozilla-central/rev/be5e92dc51ec

Comment 14

[Darkspirit]

(In reply to Jan Andre Ikenmeyer [:darkspirit] from comment #0)
> Nightly 59 x64 20171123100420 de_DE b6bed1b710c3e22cab49f22f1b5f44d80286bcb9 @ Debian Testing (KDE, Radeon RX480)

Verified fixed in Nightly 59 x64 20171124100500 de_DE f2e36fbb90fad27c034105709a4cbbaef9bd58c8 @ Debian Testing (KDE).

Comment 15

[YF (Yang)]

I see this issue in Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:59.0) Gecko/20100101 Firefox/59.0 ID:20171123220110, and Verified fixed in Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:59.0) Gecko/20100101 Firefox/59.0 ID:20171124220317.

Comment 16

[Brian Grinstead [:bgrins]]

(In reply to Brian Grinstead [:bgrins] from comment #9)
> (In reply to :Gijs from comment #8)
> > Wanna file an exploratory perf bug to look into that? :)
> 
> Sure, setting needinfo to remind myself to do that next week

Following up on this - I think we won't want to load a document sheet in the style cache as per https://bugzilla.mozilla.org/show_bug.cgi?id=1470842#c6.