Closed
Bug 1496720
Opened 6 years ago
Closed 6 years ago
[css-compat] Unship -moz/webkit-appearance values not supported by other UAs / spec
Categories
(Core :: CSS Parsing and Computation, enhancement, P3)
Core
CSS Parsing and Computation
Tracking
()
RESOLVED
FIXED
mozilla64
Tracking | Status | |
---|---|---|
firefox64 | --- | fixed |
People
(Reporter: MatsPalmgren_bugz, Assigned: MatsPalmgren_bugz)
References
(Depends on 1 open bug, Blocks 1 open bug)
Details
(Keywords: dev-doc-complete, site-compat)
Attachments
(1 file, 1 obsolete file)
We have a lot of -moz-appearance values that we're now exposing
also on -webkit-appearance since it's an alias.
We should unship these values (from both properties) by marking them:
#[parse(condition = "in_ua_or_chrome_sheet")]
https://treeherder.mozilla.org/#/jobs?repo=try&revision=d4eeea4ad9bac12627e97338c320a0dae5c99960
Assignee | ||
Comment 1•6 years ago
|
||
This seems like a safe first batch of values to unship.
We should probably also remove Scale* and Scrollbarthumb*/
Scrollbartrack* values since Chrome has slider-*/sliderthumb-*
which we should probably implement before removing the -moz-
values. And a few other values like that. (There's a more
detailed compat discussion in bug 1368555.)
Most of the values here are for XUL though, which should be
uncontroversial to unship IMO.
I'll send an intent-to-unship to dev.platform if you agree
on this list.
https://treeherder.mozilla.org/#/jobs?repo=try&revision=9625f399a221456127b0d8abeaa61e3a72cc18b7
Flags: needinfo?(jwatt)
Assignee | ||
Comment 2•6 years ago
|
||
Simon, could you run a httparchive query to get a list of -moz-appearance
values that are used in the wild? Just to make sure we're not removing
anything essential here. Would it also be possible to see if there is
a -webkit-appearance declaration in the same rule?
Flags: needinfo?(zcorpan)
Comment 3•6 years ago
|
||
I'd certainly like us to do this, but there is some potential to break our own code. I've previously come across style sheet resources that we load as resources other than ua/chrome sheets. I think it was something to do with media controls, reader mode and a few other things. I'm not sure of the problematic schemes, but possibly resource:// and about://.
(As an aside, bug 1457333 could make this less pressing.)
Assignee | ||
Comment 4•6 years ago
|
||
Do we still use XUL in our videocontrols?
Flags: needinfo?(timdream)
Comment 5•6 years ago
|
||
When do when dom.ua_widget.enabled is false, which is true for all release channels except Nightly:
https://searchfox.org/mozilla-central/rev/807a37c670c093b6e5201841a7c5315ba67ba8d5/layout/generic/nsVideoFrame.cpp#152
When that pref is true I think we don't create any XUL elements in the shadow tree, since I don't see xul stuff in:
https://searchfox.org/mozilla-central/source/toolkit/content/widgets/videocontrols.js
Flags: needinfo?(timdream)
Comment 6•6 years ago
|
||
*We do when...
Assignee | ||
Comment 7•6 years ago
|
||
OK, but that's just the subtree root box, right?
Is this the interior we use when the pref is false?:
https://searchfox.org/mozilla-central/source/toolkit/content/widgets/videocontrols.xml
chrome://global/skin/media/videocontrols.css
If so, I think we're good since I don't see any XUL / -moz-appearance in there.
Comment 8•6 years ago
|
||
Yeah, that looks right. Worth checking the datetimebox widget as well. But I just took a look and:
https://searchfox.org/mozilla-central/rev/807a37c670c093b6e5201841a7c5315ba67ba8d5/toolkit/content/widgets/datetimebox.css#9
Just took a look at your patch, it looks great, but could you add the unshipped values to layout/style/test/test_non_content_accessible_values.html?
Comment 9•6 years ago
|
||
(In reply to Emilio Cobos Álvarez (:emilio) from comment #8)
> Yeah, that looks right. Worth checking the datetimebox widget as well. But I
> just took a look and:
(And should be fine I mean :))
Assignee | ||
Comment 10•6 years ago
|
||
Great, thanks for your help.
I'll add the values to test_non_content_accessible_values.html...
FTR, I'm not seeing any obvious regressions when using Reader Mode
in my local build.
Assignee | ||
Comment 11•6 years ago
|
||
Attachment #9014916 -
Attachment is obsolete: true
Attachment #9015027 -
Flags: review?(emilio)
Assignee | ||
Updated•6 years ago
|
Attachment #9015027 -
Attachment is patch: true
Attachment #9015027 -
Attachment mime type: application/octet-stream → text/plain
Comment 12•6 years ago
|
||
Comment on attachment 9015027 [details] [diff] [review]
Unship most of the -moz-appearance values that aren't supported by -webkit-appearance in other UAs
Review of attachment 9015027 [details] [diff] [review]:
-----------------------------------------------------------------
Thanks! Please send an intent to unship for this (I need to send some as well :)).
Attachment #9015027 -
Flags: review?(emilio) → review+
Updated•6 years ago
|
Keywords: dev-doc-needed,
site-compat
Comment 13•6 years ago
|
||
(In reply to Mats Palmgren (:mats) from comment #2)
> Simon, could you run a httparchive query to get a list of -moz-appearance
> values that are used in the wild? Just to make sure we're not removing
> anything essential here. Would it also be possible to see if there is
> a -webkit-appearance declaration in the same rule?
Done: https://docs.google.com/spreadsheets/d/1-EGzSxgFrGihArz0ZuZiZ2krrN_KhUYrSzdOhQi-zrI/edit#gid=579541380&range=B3
Updated•6 years ago
|
Flags: needinfo?(zcorpan)
Assignee | ||
Comment 14•6 years ago
|
||
Thanks Simon, that's very helpful!
It seems 'radio-container' is the most used value of the ones we plan
to unship, with 1672 occurrences. But there is a -webkit-appearance:none
in 1346 of those and -webkit-appearance:button in 317 (i.e. 9 missing
a -webkit-appearance declaration in the same rule).
Next is 'window' with ~1600 occurrences, of which about ~1320 has
a -webkit-appearance declaration ('none' mostly).
Next 1261 'menulist-text', with 1250 -webkit-appearance:none.
Next 387 'menulist-button', in this case there are very few with
a -webkit-appearance declaration in the same rule for some reason.
============================
'radio-container', 'window' and 'menulist-text' have the effect of
suppressing the border (which -webkit-appearance:none doesn't).
'window' adds a grey-ish background color.
Those are likely the most visible differences between in Firefox / Chrome
for these declarations.
So my conclusion is that unshipping these values will likely make our
rendering more compatible with Chrome's.
Assignee | ||
Updated•6 years ago
|
Flags: needinfo?(jwatt)
Assignee | ||
Comment 15•6 years ago
|
||
Sent an intent-to-unship to dev-platform:
https://groups.google.com/forum/#!topic/mozilla.dev.platform/odBz2i8xnno
Comment 16•6 years ago
|
||
Pushed by mpalmgren@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/de1a5f517578
[css-compat] Unship most of the -moz-appearance values that aren't supported by -webkit-appearance in other UAs. r=emilio
Comment 17•6 years ago
|
||
bugherder |
Status: NEW → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla64
Comment 18•6 years ago
|
||
Posted site compatibility note: https://www.fxsitecompat.com/en-CA/docs/2018/most-of-firefox-specific-moz-appearance-values-have-been-removed/
Comment 19•6 years ago
|
||
Did this already landed? "@supports (-moz-appearance: meterbar)" still works in Firefox Nightly 65.0a1 (2018-10-23).
Testcase https://output.jsbin.com/yicehin/quiet
"@supports (-moz-appearance: meterbar)" is used as a new Firefox CSS hack because "@-moz-document url-prefix()" was dropped in Firefox 61, see https://github.com/4ae9b8/browserhacks/pull/187
If decided to drop also "@supports (-moz-appearance: meterbar)" then please give us any alternative for CSS hack to handle al least 3 last Forefox versions.
Assignee | ||
Comment 20•6 years ago
|
||
'meterbar' isn't one of the values we dropped (yet). You can see which
values we dropped in this bug (the green lines with a "+") here:
https://hg.mozilla.org/mozilla-central/rev/de1a5f517578#l4.11
We will deprecate 'meterbar' (in favor of 'meter') and a few other that
other UAs don't support in the future, but those require a bit more
careful analysis before we deprecate them because they are likely more
used on the web than the values we dropped here. (We'll likely start
by aliasing the values so you can use either for a transition period.)
Comment 21•6 years ago
|
||
(In reply to Binyamin from comment #19)
> Did this already landed? "@supports (-moz-appearance: meterbar)" still works
> in Firefox Nightly 65.0a1 (2018-10-23).
> Testcase https://output.jsbin.com/yicehin/quiet
>
> "@supports (-moz-appearance: meterbar)" is used as a new Firefox CSS hack
> because "@-moz-document url-prefix()" was dropped in Firefox 61, see
> https://github.com/4ae9b8/browserhacks/pull/187
>
> If decided to drop also "@supports (-moz-appearance: meterbar)" then please
> give us any alternative for CSS hack to handle al least 3 last Forefox
> versions.
Also, note that @-moz-document url-prefix() still works, as an exception, except on Nightly. It's controlled by the pref layout.css.moz-document.url-prefix-hack.enabled.
Comment 22•6 years ago
|
||
Note to docs team:
I've added a note covering this to the Fx 64 rel notes:
https://developer.mozilla.org/en-US/docs/Mozilla/Firefox/Releases/64#CSS
Any other work still needs doing.
Comment 23•6 years ago
|
||
I've added a note in the table on https://developer.mozilla.org/en-US/docs/Web/CSS/appearance for values removed in FF63 and FF64.
Updated•6 years ago
|
Keywords: dev-doc-needed → dev-doc-complete
You need to log in
before you can comment on or make changes to this bug.
Description
•