Closed Bug 1356510 Opened 4 years ago Closed 4 years ago

stylo: support the sideways-{lr,rl} and deprecated SVG 1.1 writing-mode values

Categories

(Core :: CSS Parsing and Computation, enhancement)

enhancement
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla55
Tracking Status
firefox55 --- fixed

People

(Reporter: heycam, Assigned: heycam)

References

Details

Attachments

(1 file, 4 obsolete files)

No description provided.
Summary: stylo: support the deprecated SVG 1.1 writing-mode values → stylo: support the sideways-{lr,rl} and deprecated SVG 1.1 writing-mode values
Attachment #8858597 - Flags: review?(manishearth)
Attachment #8858598 - Flags: review?(manishearth)
Attachment #8858599 - Flags: review?(manishearth)
Attachment #8858600 - Flags: review?(manishearth)
Comment on attachment 8858598 [details]
style: Add Gecko-only support for writing-mode:sideways-{lr,rl}.

https://reviewboard.mozilla.org/r/130594/#review133198

::: servo/components/style/logical_geometry.rs:32
(Diff revision 1)
>      #[cfg_attr(feature = "servo", derive(HeapSizeOf, Serialize))]
>      pub flags WritingMode: u8 {
>          const FLAG_RTL = 1 << 0,
>          const FLAG_VERTICAL = 1 << 1,
>          const FLAG_VERTICAL_LR = 1 << 2,
> -        const FLAG_SIDEWAYS = 1 << 3,
> +        const FLAG_LINE_INVERTED = 1 << 3,

nit: doc comment for this one, it's not immediately obvious what it means.
Attachment #8858598 - Flags: review?(manishearth) → review+
Comment on attachment 8858599 [details]
style: Add Gecko-only support for the deprecated SVG 1.1 writing-mode aliases.

https://reviewboard.mozilla.org/r/130596/#review133200
Attachment #8858599 - Flags: review?(manishearth) → review+
Comment on attachment 8858600 [details]
style: Make text-orientation:sideways-right an alias.

https://reviewboard.mozilla.org/r/130598/#review133202
Attachment #8858600 - Flags: review?(manishearth) → review+
Comment on attachment 8858601 [details]
Bug 1356510 - stylo: Update test expectations.

https://reviewboard.mozilla.org/r/130600/#review133204

Wish we had more wm reftests, but since sideways is a tumultuous bit of the spec right now anyway I guess it doesn't matter.
Attachment #8858601 - Flags: review?(manishearth) → review+
Comment on attachment 8858597 [details]
Bug 1356510 - stylo: Update test expectations.

https://reviewboard.mozilla.org/r/130592/#review133206

Feels a bit like overkill, but it's not the first time we've added special casing for a few properties, so I guess this is fine :

::: servo/components/style/properties/data.py:36
(Diff revision 1)
>  
>  def to_camel_case(ident):
>      return re.sub("(^|_|-)([a-z])", lambda m: m.group(2).upper(), ident.strip("_").strip("-"))
>  
>  
> +def parse_aliases(value):

Elsewhere (see the bitflags stuff for the font-variant properties in gecko.mako.rs) we just pass in a map. Perhaps we should do the same here? Then again, this is much nicer. Up to you :)
Attachment #8858597 - Flags: review?(manishearth) → review+
(In reply to Manish Goregaokar [:manishearth] from comment #11)
> Wish we had more wm reftests, but since sideways is a tumultuous bit of the
> spec right now anyway I guess it doesn't matter.

Turns out a few more failed (including some you wrote!), but yes, would be nice if there were more sideways-{lr,rl} tests.

(In reply to Manish Goregaokar [:manishearth] from comment #12)
> Elsewhere (see the bitflags stuff for the font-variant properties in
> gecko.mako.rs) we just pass in a map. Perhaps we should do the same here?
> Then again, this is much nicer. Up to you :)

Mmm, yeah.  I think the strings are nicer. :)
(In reply to Cameron McCormack (:heycam) from comment #13)
> Turns out a few more failed (including some you wrote!), but yes, would be
> nice if there were more sideways-{lr,rl} tests.

(And by that I mean, tests that were previously annotated failing, which now pass, causing the jobs to fail...)
Attachment #8858598 - Attachment is obsolete: true
Attachment #8858599 - Attachment is obsolete: true
Attachment #8858600 - Attachment is obsolete: true
Attachment #8858601 - Attachment is obsolete: true
Pushed by cmccormack@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/045bf7706bb5
stylo: Update test expectations. r=manishearth
https://hg.mozilla.org/mozilla-central/rev/045bf7706bb5
Status: NEW → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla55
Assignee: nobody → cam
You need to log in before you can comment on or make changes to this bug.