Closed Bug 1357655 Opened 6 years ago Closed 6 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: 6 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.