Closed Bug 1360508 Opened 3 years ago Closed 3 years ago

stylo: Adjust text-combine-upright properly.


(Core :: CSS Parsing and Computation, enhancement)

Not set



Tracking Status
firefox55 --- fixed


(Reporter: emilio, Unassigned)




(3 files)

Bug 1359603 added the fixup, but to the wrong place.

Style for text is not resolved during the traversal.
Blocks: 1360530
Comment on attachment 8862831 [details]
Bug 1360508: Use the parent's frame style context when handling text-combine.

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 on attachment 8862797 [details]
Bug 1360508: Adjust text-combine properly.

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.


::: servo/components/style/
(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+
Blocks: 1360221
Pushed by
Adjust text-combine properly. r=jryans
Use the parent's frame style context when handling text-combine. r=xidorn
Allow fixups on text styles to be reflected. rpending=heycam a=orange
Test expectation updates. r=emilio
Comment on attachment 8862832 [details]
Bug 1360508: Allow fixups on text styles to be reflected.
Attachment #8862832 - Flags: review?(cam) → review+
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.