Closed Bug 1357655 Opened 7 years ago Closed 7 years ago

-webkit-appearance: inherit should resolve to appearance: none when parent is undefined

Categories

(Core :: Layout: Form Controls, defect)

Product:
Core
Component:
Layout: Form Controls
Version:
55 Branch
Type:
defect
Priority:
Not set
Severity:
normal

Tracking

()

Status:
RESOLVED FIXED
Milestone:
mozilla55
Tracking Flags:
Tracking Status
firefox54 + fixed
firefox55 + fixed

People

(Reporter: karlcow, Assigned: MatsPalmgren_bugz)

References

Details

(Keywords: dev-doc-complete, Whiteboard: [webcompat])

Attachments

(4 files)

Rendering in Firefox, Safari and Opera (left to right)
46.11 KB, image/png
Details
Testcase
1.24 KB, text/html
Details
part 1 - [css-ui] Make 'none' the initial value for 'appearance' for web and UA compatibility. Add 'appearance:auto' to UA sheets for form controls, XUL etc where needed.
14.63 KB, patch
bzbarsky
: review+
gchang
: approval-mozilla-beta+
Details | Diff | Splinter Review
part 2 - [css-ui] Add an explicit 'appearance:auto' to a couple of reftest that needs it.
3.86 KB, patch
Details | Diff | Splinter Review 
This is a spin off of https://webcompat.com/issues/5910

The test case is 
<div>
  <select>
    <option value="001">001</option>
    <option value="002">002</option>    
  </select>
</div>

with 

select {
  -webkit-appearance: inherit;  
}

in Gecko, it shows a -moz-menulist widget, aka it resolves to -webkit-appearance: auto;

in WebKit and Blink, it shows nothing, aka it resolves to -webkit-appearance: none by default.


-webkit-appearance: auto resolves by default to a menulist widget (on Desktop on WebKit, Blink and Gecko)
Created a test case here
https://codepen.io/webcompat/pen/YVwXda
(I haven't tested yet on real mobile devices).
Flags: webcompat?
Flags: needinfo?(mats)
Whiteboard: [webcompat]
Attached file Testcase — 
Results in Chrome:
div: none
select with -webkit-appearance:inherit: none
button: button
button w. -webkit-appearance:initial: none

Results in Nightly:
div: auto
select with -webkit-appearance:inherit: auto
button: auto
button w. -webkit-appearance:initial: auto
Assignee: nobody → mats
Flags: needinfo?(mats)
Hmm, so this means '-webkit-appearance' in Chrome and the css-ui spec for 'appearance'
aren't reconcilable.  https://drafts.csswg.org/css-ui-4/#appearance-switching

We should probably change the spec to say 'none' is the initial value, and then
add 'appearance:auto' in all rules in our UA sheets that have a -moz-appearance
declaration.
tracking-firefox54: --- → ?
tracking-firefox55: --- → ?
Sounds about right.  Please raise the relevant spec issues...
Mats, do you want me to provide a patch for it?
Flags: needinfo?(mats)
Thanks for the kind offer, but I already have patches that seems to work:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=786ce80b6fa26eacb9718b37c7f185b17479e6aa
Flags: needinfo?(mats)
BTW, I found some XUL files with <hbox style="-moz-appearance:listbox"> and whatnot, which
doesn't work without also specifying 'appearance:auto' now that 'none' is the initial value.
I'm worried that XUL add-ons might do something similar, so I suspect we need to also add
a rule in xul.css to specify 'appearance:auto' as the default for all generic XUL elements.
Safari and Edge have the same results as Chrome for the testcase.

Filed a spec issue:
https://github.com/w3c/csswg-drafts/issues/1250
See Also: → https://github.com/w3c/csswg-drafts/issues/1250
Comment on attachment 8860099 [details] [diff] [review]
part 1 - [css-ui] Make 'none' the initial value for 'appearance' for web and UA compatibility.  Add 'appearance:auto' to UA sheets for form controls, XUL etc where needed.

I don't think you need the xul.css change.  We always apply minimal-xul.css to documents.

r=me with that bit removed.
Attachment #8860099 - Flags: review?(bzbarsky) → review+
Pushed by mpalmgren@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/0f2c055d69b2
part 1 - [css-ui] Make 'none' the initial value for 'appearance' for web and UA compatibility.  Add 'appearance:auto' to UA sheets for form controls, XUL etc where needed.  r=bz
https://hg.mozilla.org/integration/mozilla-inbound/rev/c132d492e5ba
part 2 - [css-ui] Add an explicit 'appearance:auto' to a couple of reftest that needs it.
https://hg.mozilla.org/mozilla-central/rev/0f2c055d69b2
https://hg.mozilla.org/mozilla-central/rev/c132d492e5ba
Status: NEW → RESOLVED
Closed: 7 years ago
status-firefox55: affected → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla55
Comment on attachment 8860099 [details] [diff] [review]
part 1 - [css-ui] Make 'none' the initial value for 'appearance' for web and UA compatibility.  Add 'appearance:auto' to UA sheets for form controls, XUL etc where needed.

Approval Request Comment
[Feature/Bug causing the regression]: -webkit-appearance feature (bug 1333482)
[User impact if declined]: invisible checkbox/radio controls in some rare cases
[Is this code covered by automated tests?]:yes
[Has the fix been verified in Nightly?]:no
[Needs manual test from QE? If yes, steps to reproduce]: no
[List of other uplifts needed for the feature/fix]:
[Is the change risky?]:no
[Why is the change risky/not risky?]:trivial code changes
[String changes made/needed]:none

We should take this in Beta v54 to reduce the web-compat risk for bug 1333482 /
bug 605985.
Attachment #8860099 - Flags: approval-mozilla-beta?
Depends on: 1358840
This broke TB twist lists on Mac only, see bug 1358840. We're looking into whether this can be fixed in C-C.
The 'appearance: auto' in minimal-xul.css doesn't seem to have any effect on treechildren elements, so treechildren::-moz-tree-twisty and so on have lost their native styling.
Sorry about that.  I guess the problem is that the universal selector * doesn't apply
to pseudo-elements (sigh).  I guess we have to make an exhaustive list of XUL pseudos too.
Unless we have some internal non-standard ::-moz-any-pseudo or something...
Track 54+/55+ as web compatibility issue.
status-firefox54: --- → affected
tracking-firefox54: ? → +
tracking-firefox55: ? → +
Comment on attachment 8860099 [details] [diff] [review]
part 1 - [css-ui] Make 'none' the initial value for 'appearance' for web and UA compatibility.  Add 'appearance:auto' to UA sheets for form controls, XUL etc where needed.

Fix a web compatibility issue. Beta54+. Should be in 54 beta 2.
Attachment #8860099 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
(In reply to Mats Palmgren (:mats) from comment #15)
> [Is this code covered by automated tests?]:yes
> [Has the fix been verified in Nightly?]:no
> [Needs manual test from QE? If yes, steps to reproduce]: no

Setting qe-verify- based on Mats' assessment on manual testing needs and the fact that this fix has automated coverage.
Flags: qe-verify-
Depends on: 1365627
Adding dev-doc-needed - we probably ought to mention this somewhere in the docs.
Keywords: dev-doc-needed
I have added a note about this to the appearance browser compat data:
https://developer.mozilla.org/en-US/docs/Web/CSS/appearance#Browser_compatibility

The "Initial value" info at the top of the page still says "auto", but I've submitted a PR to change this in our data repo:
https://github.com/mdn/data/pull/83

I have also added a note the Fx55 rel notes:
https://developer.mozilla.org/en-US/Firefox/Releases/55#CSS

Let me know if this looks OK, or if anything else is needed. Thanks!
Keywords: dev-doc-needed → dev-doc-complete
Chris, support for unprefixed "appearance" got backed out; see bug 1365614.

You should probably talk to Mats about what the documentation should actually say here, but I believe at this point no engine ships the "appearance" property that's in the spec, nor has any plans to.  The prefixed "-whatever-appearance" properties have totally different behavior from the spec property, and shipping the spec property in the form it's in right now is almost certainly not web-compatible...
Flags: needinfo?(cmills)
(In reply to Boris Zbarsky [:bz] (still a bit busy) (if a patch has no decent message, automatic r-) from comment #25)
> Chris, support for unprefixed "appearance" got backed out; see bug 1365614.
> 
> You should probably talk to Mats about what the documentation should
> actually say here, but I believe at this point no engine ships the
> "appearance" property that's in the spec, nor has any plans to.  The
> prefixed "-whatever-appearance" properties have totally different behavior
> from the spec property, and shipping the spec property in the form it's in
> right now is almost certainly not web-compatible...

OK, thanks Boris. I'll contact Mats offline and follow up on this; at this point I am confused as to what I really need to write.
Flags: needinfo?(cmills)
The issue on Firefox Nightly 55.0a1 (2017-05-24) (Desktop and Android)
for https://webcompat.com/issues/5910
aka https://rumble.com/register.php?src=https%3A//rumble.com/
is not solved. 

The reason is the site contains only 
	-webkit-appearance: inherit;

which is ignored for us, and the same for:

* -webkit-appearance: none; (ignored on Gecko)
* appearance: inherit; (ignored on Gecko)

The only way to fix it on their site for now is 
either to use

-moz-appearance: inherit (which resolves to none) like in WebKit/Blink world.
-moz-appearance: none

appearance: none is not working because of Bug 1365614 being backed out.
Should we reopen this one?
Depends on: 1368555
This was backed out in bug 1365614 (along with other bugs).
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: