Closed
Bug 1369198
Opened 8 years ago
Closed 8 years ago
stylo: all:inherit doesn't work with width for textareas
Categories
(Core :: CSS Parsing and Computation, enhancement)
Core
CSS Parsing and Computation
Tracking
()
RESOLVED
FIXED
mozilla55
| Tracking | Status | |
|---|---|---|
| firefox55 | --- | fixed |
People
(Reporter: manishearth, Assigned: emilio)
References
(Blocks 1 open bug)
Details
Attachments
(1 file)
<!DOCTYPE html>
<style>
textarea {
all: inherit;
width: 100%;
}
</style>
<body>
<textarea style="border: 1px solid black;">aaa</textarea>
</body>
will show a screen-width-spanning textarea in Gecko, but in Stylo it will be a default size textarea.
Presumably the way we set default sizes on anonymous inner content doesn't play well with `all:inherit`.
This happens for textarea and video, but afaict not any other elements
|
Assignee | |
Comment 1•8 years ago
|
||
So there are two problems here.
First, we don't respect the position where the "all" property appeared, and put it after it. That's easy to fix, I'll open a separate bug for it, with a test if it doesn't fix any.
With that fixed, then the problem is the deduplication we do. Given how we do the "all" property, and the internal order in the sub-properties, we end up with "inline-width: inherit" after "width: 100%". That one wins and we keep the "inherit".
But that means there's a problem with logical properties in general, as can be demonstrated by:
<!DOCTYPE html>
<style>
div {
width: 10px;
inline-size: 100px;
width: 100%;
background: red;
height: 10px;
}
</style>
<div>
</div>
That should show a 100% width div, while in stylo, inline-size wins because we deduplicate the last `width` property declaration with the first `width` property declaration that appeared. Thus, we show a 100px width div.
|
|
Assignee | |
Comment 2•8 years ago
|
||
The second one is pretty annoying. Happy to hear ideas that don't imply destroying our parsing perf :)
|
|
Assignee | |
Comment 3•8 years ago
|
||
Gah, I guess the first is not relevant after all, and for the first we can just shift the declarations...
| Comment hidden (mozreview-request) |
|
||
Comment 5•8 years ago
|
||
I don't think we can use this approach. CSSOM requires that we re-use the existing declaration: https://drafts.csswg.org/cssom/#set-a-css-declaration (as invoked by setProperty)
|
|
||
Comment 6•8 years ago
|
||
| mozreview-review | ||
Comment on attachment 8873250 [details]
Bug 1369198: Ensure pushing a declaration to a declaration block for parsing always inserts it at the last position.
https://reviewboard.mozilla.org/r/144702/#review148676
Attachment #8873250 -
Flags: review?(cam) → review-
| Comment hidden (mozreview-request) |
|
|
||
Comment 8•8 years ago
|
||
| mozreview-review | ||
Comment on attachment 8873250 [details]
Bug 1369198: Ensure pushing a declaration to a declaration block for parsing always inserts it at the last position.
https://reviewboard.mozilla.org/r/144702/#review149004
Great, thanks! :)
Attachment #8873250 -
Flags: review?(cam) → review+
Pushed by ecoal95@gmail.com:
https://hg.mozilla.org/integration/autoland/rev/86b7357e55e2
Update reftest expectations. r=emilio
|
||
Comment 10•8 years ago
|
||
| bugherder | ||
Status: NEW → RESOLVED
Closed: 8 years ago
status-firefox55:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla55
|
||
Updated•6 years ago
|
Assignee: nobody → emilio
You need to log in
before you can comment on or make changes to this bug.
Description
•