Closed Bug 1316172 Opened 3 years ago Closed 3 years ago

Add reftests for logical properties

Categories

(Core :: CSS Parsing and Computation, defect)

defect
Not set

Tracking

()

RESOLVED FIXED
mozilla52
Tracking Status
firefox52 --- fixed

People

(Reporter: manishearth, Assigned: manishearth)

References

Details

Attachments

(2 files)

Attached image Screenshot of test
Test passes locally.
Tests for logical properties should probably belong to css-logical-props rather than css-writing-modes. Also I guess jfkthame is a better reviewer for this patch.
The specific part of the spec it tests is in writing-modes though.
Probably not, though I can tweak that easily.
Comment on attachment 8808836 [details]
Bug 1316172 - Add a reftest for logical properties;

https://reviewboard.mozilla.org/r/91568/#review91854

Looks good, except for one comment:

::: layout/reftests/w3c-css/submitted/writing-modes-3/reftest-stylo.list:18
(Diff revision 3)
>  == text-combine-upright-compression-006a.html text-combine-upright-compression-006a.html
>  == text-combine-upright-compression-007.html text-combine-upright-compression-007.html
>  
>  == text-orientation-upright-directionality-001.html text-orientation-upright-directionality-001.html
> +
> +== logical-physical-mapping-001.html logical-physical-mapping-001-ref.html

This is wrong, since the reftest-stylo.list files repeat the test twice and don't use the reference.  They also say "DO NOT EDIT", which I suspect should be honored.
Attachment #8808836 - Flags: review?(dbaron) → review+
> repeat the test twice and don't use the reference

good point, fixed.

> They also say "DO NOT EDIT".

That's a lie :) It's fine to edit to add new tests, and we're not relying on these in stylo-land yet (using them as general smoketests to ensure that things are mostly still working and that there are no major regressions -- but individual test failures are still okay)

Marking as == in stylo since the relevant stuff for this to work in stylo will land soon.
Pushed by manishearth@gmail.com:
https://hg.mozilla.org/integration/autoland/rev/742ff7058a1e
Add a reftest for logical properties; r=dbaron
https://hg.mozilla.org/mozilla-central/rev/742ff7058a1e
Status: ASSIGNED → RESOLVED
Closed: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla52
Oops, the import gave:

remote: vendor-imports/mozilla/mozilla-central-reftests/writing-modes-3/logical-physical-mapping-001.html status changed to 'Needs Work' due to error:
remote: Title not specified. Linked to unversioned specification URL https://drafts.csswg.org/css-writing-modes/#logical-to-physical. Use https://www.w3.org/TR/css-writing-modes-3/... or https://drafts.csswg.org/css-writing-modes-3/... instead.
Flags: needinfo?(manishearth)
Would you mind landing the fix so it gets picked up on the next import?
Flags: needinfo?(manishearth)
Depends on: 1317092
Hmm, I think you just need to land a followup commit directly.
Flags: needinfo?(manishearth)
You need to log in before you can comment on or make changes to this bug.