Closed Bug 1496720 Opened 1 year ago Closed 1 year ago

[css-compat] Unship -moz/webkit-appearance values not supported by other UAs / spec

Categories

(Core :: CSS Parsing and Computation, enhancement, P3)

enhancement

Tracking

()

RESOLVED FIXED
mozilla64
Tracking Status
firefox64 --- fixed

People

(Reporter: mats, Assigned: mats)

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
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)
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)
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.)
Do we still use XUL in our videocontrols?
Flags: needinfo?(timdream)
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)
*We do when...
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.
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?
(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 :))
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.
Attachment #9015027 - Attachment is patch: true
Attachment #9015027 - Attachment mime type: application/octet-stream → text/plain
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+
(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
Flags: needinfo?(zcorpan)
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.
Flags: needinfo?(jwatt)
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
https://hg.mozilla.org/mozilla-central/rev/de1a5f517578
Status: NEW → RESOLVED
Closed: 1 year ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla64
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.
'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.)
(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.
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.
Depends on: 1507124
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.
You need to log in before you can comment on or make changes to this bug.