Closed Bug 1316172 Opened 3 years ago Closed 3 years ago
Add reftests for logical properties
Created a reftest in https://github.com/servo/servo/pull/14120#issuecomment-259251049 , we should import it
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.
Does this test fit within the viewport size for reftests? http://searchfox.org/mozilla-central/rev/abf500eeaf1e0b1f417789207a4b9abc4eea8c3e/layout/tools/reftest/README.txt#13-18
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 firstname.lastname@example.org: https://hg.mozilla.org/integration/autoland/rev/742ff7058a1e Add a reftest for logical properties; r=dbaron
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.
Would you mind landing the fix so it gets picked up on the next import?
Hmm, I think you just need to land a followup commit directly.
Well, it's now landed in bug 1317092.
You need to log in before you can comment on or make changes to this bug.