Closed
Bug 548648
Opened 15 years ago
Closed 15 years ago
Reftest inline--position-relative--01-ref.xhtml fails on n810, n900, mac
Categories
(Core :: SVG, defect)
Tracking
()
RESOLVED
FIXED
People
(Reporter: dholbert, Assigned: dholbert)
References
()
Details
Attachments
(2 files, 4 obsolete files)
(deleted),
image/png
|
Details | |
(deleted),
patch
|
jwatt
:
review+
|
Details | Diff | Splinter Review |
The reftest
http://mxr.mozilla.org/mozilla-central/source/layout/reftests/svg/sizing/inline--position-relative--01.xhtml
fails on mac (as noted in its reftest.list), as well as on fennec on maemo (both n900 & n810).
In each failure, there's a thin 1px-wide partly-translucent strip, just inside the solid blue border, on the left or bottom edge.
jrmuizel says this is likely because...
- the SVG content is positioned relative to text
- on mac & maemo, the default font has font glyphs that aren't exact pixel-widths.
- we don't snap SVG content to pixels, while we do snap borders & images (and the reference case uses a static image)
fantasai suggested making the test use the 'ahem' font for its blocks of text -- in that font, each glyph is a 1em-by-1em square. (This would solve the issue as long as we set 'font-size' to an exact pixel amount).
However, if we were to do that, we might as well just get rid of the text altogether, since it's being used for exact-pixel-padding.
If the reftest is trying to test pixel-snapping, then the test probably should be removed, since that's not a property we guarantee for SVG content AFAIK. However, if it's just testing relative positioning in general, then the 'ahem' font would probably be a good workaround for this subpixel issue.
Assignee | ||
Comment 1•15 years ago
|
||
Here's a screenshot that mfinkle sent me, from fennec on a maemo device.
It has a 1px-wide vertical sliver of translucency, 1px inwards from the left edge of the blue block.
Comment 2•15 years ago
|
||
Is there any other information we need to figure this out?
Assignee | ||
Comment 3•15 years ago
|
||
I don't think so... Re-reading comment 0, I think using 'ahem' is correct. I think this test is just testing relative-positioning, not pixel-snapping.
Assignee: nobody → dholbert
Status: NEW → ASSIGNED
Assignee | ||
Comment 4•15 years ago
|
||
Here's a patch to fix the test. mfinkle or jmaher, mind testing this in a reftest run on maemo?
(Note: this needs testing as part of an actual reftest run, rather than just a viewing of the static test, since it references a parent directory to get to the "ahem" font. That doesn't work when viewing the static file, and it only works in a reftest run due to the "HTTP(../..)" that I added in reftest.list)
Attachment #435677 -
Flags: review?(jwatt)
Comment 5•15 years ago
|
||
I will give it a run on maemo...thanks for the patch.
Comment 6•15 years ago
|
||
this passes on my n900 maemo device.
Comment 7•15 years ago
|
||
but I should note that there are 2 other similar bugs which are continuing to fail:
tests/layout/reftests/svg/sizing/inline--display-inline-block--01.xhtml
tests/layout/reftests/svg/sizing/inline--display-inline--01.xhtml
Assignee | ||
Comment 8•15 years ago
|
||
Ok, this version separates out the new style into an "ahem.css" file, which we'll now use in all the reftests in that directory that were marked as failing on Mac. (which are the reftests with subpixel-font-size issues, I presume)
So: this patch fixes the original reftest here and the two mentioned in comment 7, as well as the test "inline--display-inline-block--01.xhtml", so that they'll all pass on Mac & maemo. I'm running it through tryserver to test it on Mac.
Attachment #435677 -
Attachment is obsolete: true
Attachment #435917 -
Flags: review?
Attachment #435677 -
Flags: review?(jwatt)
Assignee | ||
Updated•15 years ago
|
Attachment #435917 -
Flags: review? → review?(jwatt)
Assignee | ||
Comment 9•15 years ago
|
||
(Last patch had two unintended whitespace-only changes; nixed those in this version.)
Attachment #435917 -
Attachment is obsolete: true
Attachment #435926 -
Flags: review?(jwatt)
Attachment #435917 -
Flags: review?(jwatt)
Assignee | ||
Comment 10•15 years ago
|
||
Previous patch worked on mac, but failed on Windows, apparently due to a fractional-pixel-value for line-height.
http://tinderbox.mozilla.org/showlog.cgi?log=MozillaTry/1269965653.1269975481.3175.gz
This new patch version specifies font-size and line-height as exact pixel values to fix that problem. Trying it on Try Server now.
Attachment #435926 -
Attachment is obsolete: true
Attachment #435990 -
Flags: review?(jwatt)
Attachment #435926 -
Flags: review?(jwatt)
Assignee | ||
Comment 11•15 years ago
|
||
...and of course that should be a line-height of 12px (or anything integral > 10px), not 2px... fixed here.
Attachment #435990 -
Attachment is obsolete: true
Attachment #435990 -
Flags: review?(jwatt)
Assignee | ||
Updated•15 years ago
|
Attachment #436133 -
Flags: review?(jwatt)
Comment 12•15 years ago
|
||
Comment on attachment 436133 [details] [diff] [review]
fix v2c
r=jwatt. Appologies for missing this review request earlier.
Attachment #436133 -
Flags: review?(jwatt) → review+
Assignee | ||
Comment 13•15 years ago
|
||
No worries -- thanks for the review! :) Pushed:
http://hg.mozilla.org/mozilla-central/rev/fd883c55f992
Status: ASSIGNED → RESOLVED
Closed: 15 years ago
Resolution: --- → FIXED
You need to log in
before you can comment on or make changes to this bug.
Description
•