Closed
Bug 1360508
Opened 8 years ago
Closed 7 years ago
stylo: Adjust text-combine-upright properly.
Categories
(Core :: CSS Parsing and Computation, enhancement)
Core
CSS Parsing and Computation
Tracking
()
RESOLVED
FIXED
mozilla55
Tracking | Status | |
---|---|---|
firefox55 | --- | fixed |
People
(Reporter: emilio, Unassigned)
References
Details
Attachments
(3 files)
Bug 1359603 added the fixup, but to the wrong place.
Style for text is not resolved during the traversal.
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment 4•8 years ago
|
||
mozreview-review |
Comment on attachment 8862831 [details]
Bug 1360508: Use the parent's frame style context when handling text-combine.
https://reviewboard.mozilla.org/r/134746/#review137678
I guess it's fine for now... It should be a rare case that people hit this difference.
Attachment #8862831 -
Flags: review?(xidorn+moz) → review+
Comment 5•8 years ago
|
||
mozreview-review |
Comment on attachment 8862797 [details]
Bug 1360508: Adjust text-combine properly.
https://reviewboard.mozilla.org/r/134718/#review137660
Thanks for cleaning this up, I didn't realize text had a different path. Overall, looks good, but a few notes for clarity.
::: layout/style/ServoBindingList.h:336
(Diff revision 1)
> nsIAtom* pseudoTag, bool skip_display_fixup,
> RawServoStyleSetBorrowed set)
> SERVO_BINDING_FUNC(Servo_ComputedValues_Inherit, ServoComputedValuesStrong,
> RawServoStyleSetBorrowed set,
> - ServoComputedValuesBorrowedOrNull parent_style)
> + ServoComputedValuesBorrowedOrNull parent_style,
> + bool for_text)
Could we use an enum, similar to what I did with `LengthParsingMode`[1]? Bools are harder to read at a call site, especially if people forget to add annotating comments to explain them. Enums make it much more obvious.
[1]: http://searchfox.org/mozilla-central/rev/068e6f292503df13515a52321fc3844e237bf6a9/layout/style/ServoTypes.h#75
::: servo/components/style/style_adjuster.rs:95
(Diff revision 1)
> }
>
> - /// Change writing mode of text frame for text-combine-upright.
> - /// It is safe to look at the parent's style because we are looking at
> - /// inherited properties, and ::-moz-text never matches any rules.
> - #[cfg(feature = "gecko")]
> + /// Adjust the style for text style.
> + ///
> + /// The adjustments here are a subset of the adjustments generally, because
> + /// text only inherits properties.
Could you extend this comment to note that text nodes are coming through a different path (not the usual cascade)?
Attachment #8862797 -
Flags: review?(jryans) → review+
Pushed by ecoal95@gmail.com:
https://hg.mozilla.org/integration/autoland/rev/e264da11dfbe
Adjust text-combine properly. r=jryans
https://hg.mozilla.org/integration/autoland/rev/7ac1b8bcb88f
Use the parent's frame style context when handling text-combine. r=xidorn
https://hg.mozilla.org/integration/autoland/rev/766e80198c61
Allow fixups on text styles to be reflected. rpending=heycam a=orange
https://hg.mozilla.org/integration/autoland/rev/043cdedf4903
Test expectation updates. r=emilio
Comment 7•7 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/e264da11dfbe
https://hg.mozilla.org/mozilla-central/rev/7ac1b8bcb88f
https://hg.mozilla.org/mozilla-central/rev/766e80198c61
https://hg.mozilla.org/mozilla-central/rev/043cdedf4903
Status: NEW → RESOLVED
Closed: 7 years ago
status-firefox55:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla55
Comment 8•7 years ago
|
||
mozreview-review |
Comment on attachment 8862832 [details]
Bug 1360508: Allow fixups on text styles to be reflected.
https://reviewboard.mozilla.org/r/134748/#review137848
Attachment #8862832 -
Flags: review?(cam) → review+
Updated•7 years ago
|
Summary: stylo: Adjust text-combine properly. → stylo: Adjust text-combine-upright properly.
You need to log in
before you can comment on or make changes to this bug.
Description
•