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

Don't clip range/checkbox/radio overflow to border box. #10025

Merged
merged 1 commit into from
Jan 12, 2024

Conversation

emilio
Copy link
Contributor

@emilio emilio commented Dec 27, 2023

This fixes #10024.

(See WHATWG Working Mode: Changes for more details.)


/rendering.html ( diff )

@emilio emilio requested review from zcorpan, annevk and mfreed7 December 27, 2023 07:58
@emilio
Copy link
Contributor Author

emilio commented Dec 27, 2023

We might want to give the same treatment to checkbox/radio... People use appearance: none to build custom checkboxes, and for the non-none appearance case there's no way to cause overflow. I haven't seen issues with those in the wild tho, thoughts?

moz-wptsync-bot pushed a commit to web-platform-tests/wpt that referenced this pull request Dec 29, 2023
HTML spec issue / PR: whatwg/html#10025

Differential Revision: https://phabricator.services.mozilla.com/D197327

bugzilla-url: https://bugzilla.mozilla.org/show_bug.cgi?id=1872006
gecko-commit: d854806633ea82ac653a4798573a64ee16b8483e
gecko-reviewers: dholbert
moz-v2v-gh pushed a commit to mozilla/gecko-dev that referenced this pull request Dec 29, 2023
@nt1m
Copy link
Member

nt1m commented Dec 30, 2023

@emilio I wonder if the styles should be applied on input:is([type=button i], [type=reset i], [type=submit]) instead. I would prefer avoiding applying the clip unnecessarily.

@emilio
Copy link
Contributor Author

emilio commented Dec 30, 2023

@nt1m, can you elaborate on why?

File inputs at least also clip overflow (and can generate overflow otherwise) cross-browser. Text-like inputs do also clip overflow, and due to text being the fallback type, it's non-trivial to define that via CSS (probably need to use revert).

So IIUC that leaves checkbox/radio/image/range left which don't need / have this special clipping behavior, right? Also the spec used to say "anything but image".

So not opposed I guess, if you have a better way to specify this, but :not(range, checkbox, radio, image) seems slightly preferable to me. Otherwise we need prose to define what text inputs and such do which doesn't seem amazing.

@emilio
Copy link
Contributor Author

emilio commented Dec 30, 2023

That is, from my perspective, the most we can get to explain replaced-ish controls like "(inline-)block + overflow: clip, with unspecified stuff inside", the better.

moz-wptsync-bot pushed a commit to web-platform-tests/wpt that referenced this pull request Dec 31, 2023
HTML spec issue / PR: whatwg/html#10025

Differential Revision: https://phabricator.services.mozilla.com/D197327

bugzilla-url: https://bugzilla.mozilla.org/show_bug.cgi?id=1872006
gecko-commit: d854806633ea82ac653a4798573a64ee16b8483e
gecko-reviewers: dholbert
This is not compatible since appearance: none can make these elements
create overflow in a way such that engines honor the overflow property.

These also don't create any overflow unless they are appearance: none,
so they don't need the magic clip prose anyways.

Fixes whatwg#10024.
@emilio
Copy link
Contributor Author

emilio commented Dec 31, 2023

#10025 (comment) was indeed a real issue, see https://bugzilla.mozilla.org/show_bug.cgi?id=1872509, so I've updated the PR accordingly.

moz-v2v-gh pushed a commit to mozilla/gecko-dev that referenced this pull request Dec 31, 2023
Since it's not web-compatible. I had already brought this possibility up
in the comments of whatwg/html#10025, so will
update that PR accordingly.

Remove forceful overflow: hidden for file / date inputs, which was
caught by the tests in the next patch.

Differential Revision: https://phabricator.services.mozilla.com/D197456
gecko-dev-updater pushed a commit to marco-c/gecko-dev-wordified-and-comments-removed that referenced this pull request Jan 1, 2024
HTML spec issue / PR: whatwg/html#10025

Differential Revision: https://phabricator.services.mozilla.com/D197327

UltraBlame original commit: d854806633ea82ac653a4798573a64ee16b8483e
gecko-dev-updater pushed a commit to marco-c/gecko-dev-wordified-and-comments-removed that referenced this pull request Jan 1, 2024
Since it's not web-compatible. I had already brought this possibility up
in the comments of whatwg/html#10025, so will
update that PR accordingly.

Remove forceful overflow: hidden for file / date inputs, which was
caught by the tests in the next patch.

Differential Revision: https://phabricator.services.mozilla.com/D197456

UltraBlame original commit: 30b0d1ecdc26a3e4753bef7807739106282ed827
gecko-dev-updater pushed a commit to marco-c/gecko-dev-wordified that referenced this pull request Jan 1, 2024
HTML spec issue / PR: whatwg/html#10025

Differential Revision: https://phabricator.services.mozilla.com/D197327

UltraBlame original commit: d854806633ea82ac653a4798573a64ee16b8483e
gecko-dev-updater pushed a commit to marco-c/gecko-dev-wordified that referenced this pull request Jan 1, 2024
Since it's not web-compatible. I had already brought this possibility up
in the comments of whatwg/html#10025, so will
update that PR accordingly.

Remove forceful overflow: hidden for file / date inputs, which was
caught by the tests in the next patch.

Differential Revision: https://phabricator.services.mozilla.com/D197456

UltraBlame original commit: 30b0d1ecdc26a3e4753bef7807739106282ed827
gecko-dev-updater pushed a commit to marco-c/gecko-dev-comments-removed that referenced this pull request Jan 1, 2024
HTML spec issue / PR: whatwg/html#10025

Differential Revision: https://phabricator.services.mozilla.com/D197327

UltraBlame original commit: d854806633ea82ac653a4798573a64ee16b8483e
gecko-dev-updater pushed a commit to marco-c/gecko-dev-comments-removed that referenced this pull request Jan 1, 2024
Since it's not web-compatible. I had already brought this possibility up
in the comments of whatwg/html#10025, so will
update that PR accordingly.

Remove forceful overflow: hidden for file / date inputs, which was
caught by the tests in the next patch.

Differential Revision: https://phabricator.services.mozilla.com/D197456

UltraBlame original commit: 30b0d1ecdc26a3e4753bef7807739106282ed827
@emilio emilio requested a review from nt1m January 1, 2024 10:48
@emilio emilio changed the title Don't clip range overflow to border box. Don't clip range/checkbox/radio overflow to border box. Jan 1, 2024
vinnydiehl pushed a commit to vinnydiehl/mozilla-unified that referenced this pull request Jan 3, 2024
vinnydiehl pushed a commit to vinnydiehl/mozilla-unified that referenced this pull request Jan 3, 2024
Since it's not web-compatible. I had already brought this possibility up
in the comments of whatwg/html#10025, so will
update that PR accordingly.

Remove forceful overflow: hidden for file / date inputs, which was
caught by the tests in the next patch.

Differential Revision: https://phabricator.services.mozilla.com/D197456
@@ -131683,7 +131683,7 @@ input, select, button, textarea {
appearance: auto;
}

input:not([type=image i]) {
input:not([type=image i], [type=range i], [type=checkbox i], [type=radio i]) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems reasonable to me, assuming your compat analysis shows that this is web compatible.

@annevk
Copy link
Member

annevk commented Jan 9, 2024

@emilio data:text/html,<input%20type=file%20style=width:10px;height:10px> does not clip for me in Safari. In fact, WebKit has code that ensures all form controls get painted correctly when their CSS box is smaller than the size they need for painting.

@annevk
Copy link
Member

annevk commented Jan 9, 2024

cc @pxlcoder

@emilio
Copy link
Contributor Author

emilio commented Jan 9, 2024

@emilio data:text/html,<input%20type=file%20style=width:10px;height:10px> does not clip for me in Safari. In fact, WebKit has code that ensures all form controls get painted correctly when their CSS box is smaller than the size they need for painting.

That doesn't match the spec either before or after my change tho, right? Gecko has always clipped file inputs, and forcefully since https://bugzilla.mozilla.org/show_bug.cgi?id=1543477 (to match Chromium and the spec).

@annevk
Copy link
Member

annevk commented Jan 9, 2024

I think with respect to #9994 that's true, but the claim above about it happening across browsers is not. And as you are now suggesting normative changes I do think it warrants consideration.

@emilio
Copy link
Contributor Author

emilio commented Jan 9, 2024

Yeah, I think I mis-tested Webkit, thanks for pointing that out. However I think this is still a progression (and the change proposed doesn't change behavior wrt file inputs in particular, fwiw).

@annevk
Copy link
Member

annevk commented Jan 9, 2024

I think what I'd like is that as part of this change we add test coverage for all types and get agreement that's where we want to end up. So we don't keep changing this around. And for WebKit that should probably be decided by @pxlcoder and @nt1m among others.

@emilio
Copy link
Contributor Author

emilio commented Jan 9, 2024

Tests in https://phabricator.services.mozilla.com/D197457 cover all input types, fwiw. Other than that, I agree. I think barring any further compat issues (which are, IMO, unlikely) this is a much better state to be in than pre - #9994, and is a step to have beter interop here.

@annevk
Copy link
Member

annevk commented Jan 9, 2024

If we do this as-is and thus agree clipping is preferred, #9546 (switch) will need to add , input[type=checkbox i][switch] presumably.

@emilio
Copy link
Contributor Author

emilio commented Jan 9, 2024

If we do this as-is and thus agree clipping is preferred, #9546 (switch) will need to add , input[type=checkbox i][switch] presumably.

You mean set overflow: clip explicitly on input[type=checkbox][switch]? I'm not convinced you'd want that.

In general I don't think "clipping is preferred" is correct. Just "clipping is the long-standing interoperable behavior for most input types, and this defines it in a way that integrates correctly with other CSS features, instead of magic prose".

@emilio
Copy link
Contributor Author

emilio commented Jan 9, 2024

It also happens to be the case that a negation is the easiest (only?) way to target text inputs with the rule, given the "random input type falls back to text" behavior.

@annevk
Copy link
Member

annevk commented Jan 11, 2024

I discussed this with @pxlcoder and we're okay with going in this direction. We haven't fully evaluated yet if this is possible for all controls, but making incremental progress is okay with us. Does that work?

If so, I can merge this.

@emilio
Copy link
Contributor Author

emilio commented Jan 12, 2024

That sounds good. I'll push the tests then. Thanks Anne!

@annevk
Copy link
Member

annevk commented Jan 12, 2024

Hmm, I was about to merge this, but upon rereading

These also don't create any overflow unless they are appearance: none, so they don't need the magic clip prose anyways.

strikes me as incorrect for WebKit on macOS which does have its own minimal sizes for controls for painting purposes. Would that still require magic clip prose then?

@emilio
Copy link
Contributor Author

emilio commented Jan 12, 2024

strikes me as incorrect for WebKit on macOS which does have its own minimal sizes for controls for painting purposes. Would that still require magic clip prose then?

Not really right? My understanding is that you're not clipping it, so this would preserve your current behaviour.

@annevk
Copy link
Member

annevk commented Jan 12, 2024

Ah right, but I should remove the remark as they can cause overflow in some implementations for appearance:auto.

@annevk annevk merged commit 1b64254 into whatwg:main Jan 12, 2024
2 checks passed
@emilio emilio deleted the range-overflow branch January 15, 2024 09:21
marcoscaceres pushed a commit to web-platform-tests/wpt that referenced this pull request Feb 23, 2024
HTML spec issue / PR: whatwg/html#10025

Differential Revision: https://phabricator.services.mozilla.com/D197327

bugzilla-url: https://bugzilla.mozilla.org/show_bug.cgi?id=1872006
gecko-commit: d854806633ea82ac653a4798573a64ee16b8483e
gecko-reviewers: dholbert
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

[forms] Range input shouldn't clip its contents to the border box
4 participants