Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Should we do percentage height quirk for calc()? #48

Open
xiaochengh opened this issue Dec 12, 2019 · 19 comments
Open

Should we do percentage height quirk for calc()? #48

xiaochengh opened this issue Dec 12, 2019 · 19 comments
Labels
good first issue Ideal for someone new to a WHATWG standard or software project

Comments

@xiaochengh
Copy link

The spec applies percentage height quirk only when

... the specified value for the height property of element is a <percentage>, ...

which means we should apply the quirk for % heights but shouldn't for calc(length + %) heights.

Currently, only Firefox does that. Chrome, EdgeHTML and Safari apply the quirk also on calc() heights.

Should we change the spec to match the behavior of Chrome & Edge & Safari?

Also @bfgeek @bzbarsky @lilles from the related Chromium bug https://crbug.com/388892

@emilio
Copy link

emilio commented Dec 12, 2019

I haven't seen any compat issues with that, fwiw. And I think making calc() quirky would be unfortunate unless we see them.

@bfgeek
Copy link
Member

bfgeek commented Dec 12, 2019

For things like this we often see compat issues with mobile specific sites.

Also FWIW Firefox actually does support this, e.g.
https://www.software.hixie.ch/utilities/js/live-dom-viewer/?saved=7544
:P

This is obviously due to the engine simplifying calc(100% + 0px) to 100%, however this super confusing to the web developer who expects calc(100% + 0px) and calc(100% + 1px) to act in the same way.

@emilio Given the above any objection to changing the spec&Firefox to match the other implementations?

@emilio
Copy link

emilio commented Dec 12, 2019

Per spec calc(% + 0px) behaves the same as %. So no, it's not quite the same than when you're mixing actual lengths.

@emilio
Copy link

emilio commented Dec 12, 2019

And I intentionally changed the behavior of that particular test-case in https://bugzilla.mozilla.org/show_bug.cgi?id=1577139, per the resolution in w3c/csswg-drafts#3482.

@bfgeek
Copy link
Member

bfgeek commented Dec 12, 2019

But you agree that as far as the web-developer is concerned, the layout result of calc(x% + y px) should behave the same way irregardless of the the values of x and y?

@emilio
Copy link

emilio commented Dec 12, 2019

That is not always compatible with the above discussion, see http://w3c-test.org/css/css-tables/calc-percent-plus-0px-auto.html for a simple example where Chromium agrees with Firefox.

I don't feel too strongly about this, but I think that lacking evidence that it can cause compat problems, introducing quirks in calc() expressions is a bit unfortunate.

@emilio
Copy link

emilio commented Dec 12, 2019

I'm ok if we do this in the name of "shortest path to interop", but I think this is a bit sad :/

@bfgeek
Copy link
Member

bfgeek commented Dec 12, 2019

I'm pretty convinced this more explainable/sane to web-developers than what is currently spec'd given above, but can dub this "shortest path to interop" if you like :).

@zcorpan
Copy link
Member

zcorpan commented Dec 12, 2019

The intent was to not apply the quirk when calc() is used, full stop.

For quirks, optimizing for explainability to web developers is a non-goal. Web developers shouldn't use quirks mode and don't need to understand them.

The goals are https://quirks.spec.whatwg.org/#goals

[snip the first two, since we know the quirk as a whole is needed for web compat]

Get interoperability on quirks that are needed for Web compatibility.

We don't currently have interop on this quirk.

Where possible, limit quirks to a fixed set of legacy features so they don’t propagate into new features.

This is what excluding calc() tries to do.

I don't understand the details of how calc() works to say whether what the spec requires is implementable in a sane way. If calc(100%) should be equivalent to 100% for all other purposes, but still needs different behavior here, I guess that requires a flag on <percentage> or so to be able to tell if it came from a calc?

So I think there are two paths:

  1. Everyone changes their implementation to match the spec.
  2. Gecko changes their implementation to match webkit/chromium, and we update the spec. @emilio is sad.

(2) is less effort overall and maybe avoids some implementation complexity (?), but we miss out on the goal to limit this quirk.

@annevk
Copy link
Member

annevk commented Dec 12, 2019

I wonder what @smfr thinks. (Personally I'm in agreement with @zcorpan and @emilio that quirks are best scoped to the smallest possible thing rather than increase in scope over time.)

@smfr
Copy link

smfr commented Dec 12, 2019

I agree with scoping the quirk down, and I'm good with the outcome of w3c/csswg-drafts#3482.

@bfgeek
Copy link
Member

bfgeek commented Dec 12, 2019

@emilio Firefox isn't consistent with itself wrt. heights here - for example:
https://www.software.hixie.ch/utilities/js/live-dom-viewer/?saved=7546
https://www.software.hixie.ch/utilities/js/live-dom-viewer/?saved=7547

@zcorpan Implementing this non-quirk quirk is surprisingly(!) difficult in our new (LayoutNG) implementation. There are now 4 different percentages which heights can resolve against if this is the case.

  • quirky non-replaced
  • quirky replaced
  • non-quirky non-replaced
  • non-quirky replaced

I'm not sure I agree that "explainability to web developers is a non-goal". Maybe it should be? At the end of the day web-developers are our end-users/customers, and would be sad if we made these types of quirks on the web even more confusing.

@smfr I'm happy for the WebKit team to investigate the potential compat issues. I can mark our bug as blocking on any WebKit behaviour change.

@emilio
Copy link

emilio commented Dec 12, 2019

@emilio Firefox isn't consistent with itself wrt. heights here - for example:
https://www.software.hixie.ch/utilities/js/live-dom-viewer/?saved=7546
https://www.software.hixie.ch/utilities/js/live-dom-viewer/?saved=7547

Can you elaborate? What should I see? I know of differences with other browsers related to percentages inside table cells, like https://bugzilla.mozilla.org/show_bug.cgi?id=1542098. I think that's what is going on in that test-case. But that's not related to this quirk at all.

@emilio
Copy link

emilio commented Dec 12, 2019

Ah you're totally right. We use HasPercent in one case (the replaced elements quirk) and ConvertsToPercent() in another...

It seems the HasPercent() call (which causes the discrepancy) was introduced all the way back to https://bugzilla.mozilla.org/show_bug.cgi?id=585715.

@zcorpan
Copy link
Member

zcorpan commented Dec 12, 2019

Implementing this non-quirk quirk is surprisingly(!) difficult in our new (LayoutNG) implementation.

That's certainly a good argument against special-casing calc. Limiting quirks is a nice goal, but not at any cost.

@smfr and @emilio what is the implementation complexity in WebKit and Gecko/Servo to exclude all calc()s?

@emilio
Copy link

emilio commented Dec 12, 2019

It's not terribly complex. we intentionally don't differentiate between a calc or not since https://bugzilla.mozilla.org/show_bug.cgi?id=1577139, though we could re-introduce it...

That being said, I think given Firefox is inconsistent, and Blink and WebKit do treat calc-with-percentages the same as percentages, I think I'm with @bfgeek that the esiest thing is for Firefox to change. It kinda sucks, but the behavior for replaced elements has been consistent across all browsers since calc() existed...

@zcorpan
Copy link
Member

zcorpan commented May 6, 2020

The chromium issue was closed:

Marking as WontFix as FF behaviour now appears to match Blink/WebKit.

https://bugs.chromium.org/p/chromium/issues/detail?id=388892#c22

@zcorpan
Copy link
Member

zcorpan commented May 6, 2020

I think the change would be to replace

the specified value for the height property of element is a <percentage>,

with something like (cc @tabatkins )

the specified value for the height property of element is a <percentage> or a math function containing a <percentage> or <length-percentage>

Here's the relevant test: https://github.com/web-platform-tests/wpt/blob/master/quirks/percentage-height-calculation.html

@zcorpan zcorpan added the good first issue Ideal for someone new to a WHATWG standard or software project label May 6, 2020
@tabatkins
Copy link

Yes, that should work. It's a little handwavey, but understandable.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
good first issue Ideal for someone new to a WHATWG standard or software project
Development

No branches or pull requests

7 participants