Closed
Bug 1105137
Opened 9 years ago
Closed 9 years ago
The text appears behind the image
Categories
(Core :: Layout, defect)
Tracking
()
RESOLVED
FIXED
mozilla37
People
(Reporter: over68, Assigned: smontagu)
References
Details
(Keywords: regression)
Attachments
(3 files, 1 obsolete file)
|
1.93 KB,
text/html
|
Details | |
|
21.44 KB,
patch
|
jfkthame
:
review+
Sylvestre
:
approval-mozilla-aurora+
|
Details | Diff | Splinter Review |
|
5.39 KB,
patch
|
jfkthame
:
review+
|
Details | Diff | Splinter Review |
The text appears behind the image in page https://dl.dropboxusercontent.com/u/95157096/85f61cf7/ItJnkL8E9x.html. Screenshot https://dl.dropboxusercontent.com/u/95157096/85f61cf7/Qxj3jZf7uG.png.
This problem occurs since the version 36.0a1 (2014-10-23). https://hg.mozilla.org/mozilla-central/rev/88adcf8fef83
|
||
Comment 2•9 years ago
|
||
[Tracking Requested - why for this release]: Regression window(m-i) Good: https://hg.mozilla.org/integration/mozilla-inbound/rev/506226a2e6c0 Mozilla/5.0 (Windows NT 6.1; WOW64; rv:36.0) Gecko/20100101 Firefox/36.0 ID:20141021202933 Bad: https://hg.mozilla.org/integration/mozilla-inbound/rev/f1d7da5ff1ed Mozilla/5.0 (Windows NT 6.1; WOW64; rv:36.0) Gecko/20100101 Firefox/36.0 ID:20141021211530 Pushlog: https://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?fromchange=506226a2e6c0&tochange=f1d7da5ff1ed Regressed by: Bug 1062963
Blocks: 1062963
Status: UNCONFIRMED → NEW
status-firefox35:
--- → unaffected
status-firefox36:
--- → affected
tracking-firefox36:
--- → ?
Component: General → Layout
Ever confirmed: true
Keywords: regression
|
||
Comment 3•9 years ago
|
||
(In reply to blinky from comment #0) > The text appears behind the image in page > https://dl.dropboxusercontent.com/u/95157096/85f61cf7/ItJnkL8E9x.html. This link simply redirects to a live website that looks likely to change at any time. A persistent (and preferably reduced!) testcase would be great.
|
|
||
Updated•9 years ago
|
Keywords: testcase-wanted
I think this bug occurs with the "direction: rtl", See https://dl.dropboxusercontent.com/u/95157096/85f61cf7/1DVYf03H9E.png. Without the "direction: rtl" https://dl.dropboxusercontent.com/u/95157096/85f61cf7/wjrN2jXaQQ.png.
|
|
||
Comment 5•9 years ago
|
||
Text overlaps a image same regression range.
|
Assignee | |
Updated•9 years ago
|
Assignee: nobody → smontagu
Flags: needinfo?(smontagu)
|
|
Assignee | |
Comment 7•9 years ago
|
||
This is more or less the minimal fix for the bug, but I wonder if it would make sense to add another logical class, LogicalDimension or some such, which just contains an inline and a block nscoord, and doesn't use a container width for any conversions. WDYT? The overloading of LogicalPoint for cases where it's an offset, not a point in any coordinate space, has confused me in the past -- see bug 1089581 comment 2.
Attachment #8534402 -
Flags: review?(jfkthame)
|
|
Assignee | |
Comment 8•9 years ago
|
||
Attachment #8534404 -
Flags: review?(jfkthame)
|
|
||
Comment 9•9 years ago
|
||
Comment on attachment 8534404 [details] [diff] [review] Reftest based on the testcase Review of attachment 8534404 [details] [diff] [review]: ----------------------------------------------------------------- ::: layout/reftests/bugs/1105137-1.html @@ +15,5 @@ > + <body> > + <div dir="ltr"> > + <p>bla bla bla bla bla bla bla bla bla bla bla bla bla bla bla bla bla bla bla bla bla bla bla bla bla bla bla bla bla bla bla bla bla bla bla bla bla bla bla bla bla bla bla bla bla bla bla bla bla bla bla bla bla bla bla bla bla bla bla bla bla bla bla bla bla bla bla bla bla bla </p> > + <div class="float"> > + <img src="https://bugzilla.mozilla.org/skins/standard/index/help.png" width="240" height="180"> This looks cool when viewing the test, but for landing in the tree we shouldn't refer to an external resource. Either use a local image, or omit it altogether: if you just say <img src="foo" width="240" height="180"> the test will still work, it'll just display a broken-image icon in the frame.
|
|
||
Comment 10•9 years ago
|
||
Comment on attachment 8534402 [details] [diff] [review] Treat nsFloatManager's mOrigin as an offset, not a point, and rename it to mOffset to make that clearer Review of attachment 8534402 [details] [diff] [review]: ----------------------------------------------------------------- LGTM. If you'd like to add a followup patch to introduce a new type, and switch mOffset over to use it, I'd be fine with that too. I don't much like LogicalDimension as a name, though; I'd suggest LogicalOffset or LogicalVector as possibilities - wdyt?
Attachment #8534402 -
Flags: review?(jfkthame) → review+
|
|
Assignee | |
Comment 11•9 years ago
|
||
Changed to use <img src="foo" ...> Amusingly enough, that forced me to make another change and force the direction of the image, because the broken-image icon is direction-dependent ;-)
Attachment #8534404 -
Attachment is obsolete: true
Attachment #8534404 -
Flags: review?(jfkthame)
Attachment #8534462 -
Flags: review?(jfkthame)
|
|
||
Updated•9 years ago
|
Attachment #8534462 -
Flags: review?(jfkthame) → review+
|
|
||
Updated•9 years ago
|
Keywords: testcase-wanted
|
|
Assignee | |
Comment 12•9 years ago
|
||
https://hg.mozilla.org/integration/mozilla-inbound/rev/1a723ead9844 https://hg.mozilla.org/integration/mozilla-inbound/rev/824175640caf
Flags: in-testsuite+
https://hg.mozilla.org/mozilla-central/rev/1a723ead9844 https://hg.mozilla.org/mozilla-central/rev/824175640caf
Status: NEW → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla37
|
|
Assignee | |
Comment 14•9 years ago
|
||
Comment on attachment 8534402 [details] [diff] [review] Treat nsFloatManager's mOrigin as an offset, not a point, and rename it to mOffset to make that clearer Approval Request Comment [Feature/regressing bug #]: Bug 1062963 [User impact if declined]: Visible regressions on right-to-left pages with text concealed behind floats [Describe test coverage new/current, TBPL]: Baked on trunk since 2014-12-11, reftest included in checkin [Risks and why]: low-risk, only change is to the coordinates of layout objects. [String/UUID change made/needed]: N/A
Attachment #8534402 -
Flags: approval-mozilla-aurora?
|
|
Assignee | |
Comment 15•9 years ago
|
||
Hmmm, I think the original testcase from comment 0 still shows a bug, although attachment 8529100 [details] and the testcase from comment 4 do seem to be fixed.
Flags: needinfo?(alice0775)
|
||
Updated•9 years ago
|
status-firefox37:
--- → fixed
|
|
||
Updated•9 years ago
|
Attachment #8534402 -
Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
|
|
Assignee | |
Comment 16•9 years ago
|
||
https://hg.mozilla.org/releases/mozilla-aurora/rev/19857a2ad6f5 https://hg.mozilla.org/releases/mozilla-aurora/rev/0f00bfa7e7ec
|
|
||
Comment 17•9 years ago
|
||
(In reply to Simon Montagu :smontagu from comment #15) > Hmmm, I think the original testcase from comment 0 still shows a bug, > although attachment 8529100 [details] and the testcase from comment 4 do > seem to be fixed. Filed Bug 1113526
Flags: needinfo?(alice0775)
|
||
Comment 18•9 years ago
|
||
This was backed out from Fx37 as part of addressing bug 1114329.
You need to log in
before you can comment on or make changes to this bug.
Description
•