www.download.com has scrambled text

VERIFIED FIXED in mozilla1.9beta2

Status

()

VERIFIED FIXED
12 years ago
12 years ago

People

(Reporter: Peter6, Assigned: dholbert)

Tracking

({regression, testcase})

Trunk
mozilla1.9beta2
x86
All
regression, testcase
Points:
---
Bug Flags:
blocking1.9 +
in-testsuite +

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [inline-block + positioning; page compat][dbaron-1.9:RwCo], URL)

Attachments

(5 attachments, 3 obsolete attachments)

(Reporter)

Description

12 years ago
Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9a9pre) Gecko/2007100512 Minefield/3.0a9pre ID:2007100512

repro:
open url
look at the middle-top part

result:
text on top of other text

this works fin in FF 2.0.0.7
(Reporter)

Comment 1

12 years ago
Posted image screenshot
Posted file unminimized testcase (obsolete) —
This is where I stopped minimizing.
When I remove the display: inline-block; rule from this unminized testcase, trunk looks the same as branch.
So I suspect this regressed when bug 9458 got fixed. Peter already gave an indication for that, he said on irc it regressed at/around 2007-01-27 (which is when bug 9458 got fixed).

So this bug is likely a tech evangelism issue.
Blocks: 9458

Comment 4

12 years ago
Posted file reduced testcase
Opera 9.5b, Safari 3b(win) and IE7 match on this testcase, current trunk is different.

j.j.
This probably should live somewhere in layout...
Posted file testcase2
Jj, thanks for that testcase, I already wondered why it displayed fine in Opera. I guess I should have taken a closer look at that.
Attachment #283762 - Attachment is obsolete: true
Component: General → Layout
Flags: blocking1.9?
Keywords: testcase
QA Contact: general → layout
Flags: blocking1.9? → blocking1.9+
Whiteboard: [inline-block + positioning; page compat] → [inline-block + positioning; page compat][dbaron-1.9:RwCo]
(Assignee)

Comment 7

12 years ago
Posted file testcase3
The problem is that we aren't letting an inline-block with the properties
   position: relative;
   display: inline-block; 
be a containing block for absolute content, when we should.

However, we do (correctly) let it be a containing block if we remove the "display: inline-block", or if we change "relative" to "absolute".
(Assignee)

Comment 8

12 years ago
Posted patch patch v1 (obsolete) — Splinter Review
This seems to fix it... not sure if it's exactly correct, or if there's anything more we need to add for this.
(Assignee)

Comment 9

12 years ago
Posted patch patch v2 (obsolete) — Splinter Review
This patch
1. Adds IsBlockInside() calls, to clean up code and add support for inline-block relatively-positioned frames
2. Calls NS_NewFloatingItemWrapperFrame to provide  NS_BLOCK_SPACE_MGR and NS_BLOCK_MARGIN_ROOT flags for these frames.

Note: #2 is admittedly a bit hack-ish, since we're working with relatively-positioned frames and not floats, but it seemed cleaner to me than the alternative ways for getting those flags in there.  Those alternatives being...
  a) adding a "flags" argument to NS_NewRelativeItemWrapperFrame
  b) casting newFrame as a nsBlockFrame so I could call SetFlags() on it.

Do let me know if these (or other) alternatives are preferable to calling NS_NewFloatingItemWrapperFrame, though.
Assignee: nobody → dholbert
Attachment #285296 - Attachment is obsolete: true
Status: NEW → ASSIGNED
Attachment #285300 - Flags: review?(bzbarsky)
Comment on attachment 285300 [details] [diff] [review]
patch v2

>Index: nsCSSFrameConstructor.cpp
>+        newFrame = NS_NewFloatingItemWrapperFrame(mPresShell, aStyleContext);

I'd prefer either of the other alternatives to this.  Adding an arg to NS_NewRelativeItemWrapperFrame is probably cleanest, but if you want to use a direct SetFlags call that's fine too.  There's no need to cast newFrame before calling SetFlags(), by the way.

r+sr=bzbarsky with that changed.

Add reftests for this when you land, if possible?
Attachment #285300 - Flags: superreview+
Attachment #285300 - Flags: review?(bzbarsky)
Attachment #285300 - Flags: review+
(Assignee)

Comment 11

12 years ago
Posted patch patch v3Splinter Review
Ok, this version adds an arg to NS_NewRelativeItemWrapperFrame.

Also including 4 reftests, which assert that relative + absolute-positioned divs are containers for absolute content, whether they're "display: inline-block" or not.  

Of these tests, only test "398797-1a.html" (relative + inline-block) failed before this patch, whereas tests b, c, and d all passed. I'm including b/c/d for thoroughness, though.

(These tests all pass post-patching, of course.)
(Assignee)

Updated

12 years ago
Attachment #285401 - Flags: superreview?(bzbarsky)
Attachment #285401 - Flags: review?(bzbarsky)
(Assignee)

Updated

12 years ago
Attachment #285300 - Attachment is obsolete: true
Comment on attachment 285401 [details] [diff] [review]
patch v3

s/red/yellow/ throughout (because red means fail, usually) and looks great.
Attachment #285401 - Flags: superreview?(bzbarsky)
Attachment #285401 - Flags: superreview+
Attachment #285401 - Flags: review?(bzbarsky)
Attachment #285401 - Flags: review+
(Assignee)

Updated

12 years ago
Whiteboard: [inline-block + positioning; page compat][dbaron-1.9:RwCo] → [inline-block + positioning; page compat][dbaron-1.9:RwCo][needs landing]
(Assignee)

Comment 13

12 years ago
patch v3 checked in, with s/red/yellow in the reftests.

Checking in base/nsCSSFrameConstructor.cpp;
/cvsroot/mozilla/layout/base/nsCSSFrameConstructor.cpp,v  <--  nsCSSFrameConstructor.cpp
new revision: 1.1420; previous revision: 1.1419
done
Checking in generic/nsHTMLParts.h;
/cvsroot/mozilla/layout/generic/nsHTMLParts.h,v  <--  nsHTMLParts.h
new revision: 3.149; previous revision: 3.148
done
RCS file: /cvsroot/mozilla/layout/reftests/bugs/398797-1-ref.html,v
done
Checking in reftests/bugs/398797-1-ref.html;
/cvsroot/mozilla/layout/reftests/bugs/398797-1-ref.html,v  <--  398797-1-ref.html
initial revision: 1.1
done
RCS file: /cvsroot/mozilla/layout/reftests/bugs/398797-1a.html,v
done
Checking in reftests/bugs/398797-1a.html;
/cvsroot/mozilla/layout/reftests/bugs/398797-1a.html,v  <--  398797-1a.html
initial revision: 1.1
done
RCS file: /cvsroot/mozilla/layout/reftests/bugs/398797-1b.html,v
done
Checking in reftests/bugs/398797-1b.html;
/cvsroot/mozilla/layout/reftests/bugs/398797-1b.html,v  <--  398797-1b.html
initial revision: 1.1
done
RCS file: /cvsroot/mozilla/layout/reftests/bugs/398797-1c.html,v
done
Checking in reftests/bugs/398797-1c.html;
/cvsroot/mozilla/layout/reftests/bugs/398797-1c.html,v  <--  398797-1c.html
initial revision: 1.1
done
RCS file: /cvsroot/mozilla/layout/reftests/bugs/398797-1d.html,v
done
Checking in reftests/bugs/398797-1d.html;
/cvsroot/mozilla/layout/reftests/bugs/398797-1d.html,v  <--  398797-1d.html
initial revision: 1.1
done
RCS file: /cvsroot/mozilla/layout/reftests/bugs/398797-style.css,v
done
Checking in reftests/bugs/398797-style.css;
/cvsroot/mozilla/layout/reftests/bugs/398797-style.css,v  <--  398797-style.css
initial revision: 1.1
done
Checking in reftests/bugs/reftest.list;
/cvsroot/mozilla/layout/reftests/bugs/reftest.list,v  <--  reftest.list
new revision: 1.188; previous revision: 1.187
done
Status: ASSIGNED → RESOLVED
Last Resolved: 12 years ago
Flags: in-testsuite+
OS: Windows XP → All
Resolution: --- → FIXED
(Assignee)

Comment 14

12 years ago
Reftest fails on mac for some reason -- the reference case doesn't have a large enough yellow background rectangle on the "bar" div.

Marked reftest as being random on cocoa for now, to fix tinderbox orangeness.  I'm investigating, and I'll either fix the issue or file a new bug for it.
(Assignee)

Comment 15

12 years ago
(In reply to comment #14)
> Reftest fails on mac for some reason

Windows too, actually.

> I'm investigating, and I'll either fix the issue or file a new bug for it.

Done -- filed bug 402918.

In the meantime, I set "float: left" on the reference case's <span>, and that works around the problem.  Added a comment to explain this and to point to bug 402918.
Target Milestone: --- → mozilla1.9 M10
verified fixed using Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9b2pre) Gecko/2007112805 Minefield/3.0b2pre, download.com site renders fine.
Status: RESOLVED → VERIFIED
Whiteboard: [inline-block + positioning; page compat][dbaron-1.9:RwCo][needs landing] → [inline-block + positioning; page compat][dbaron-1.9:RwCo]
You need to log in before you can comment on or make changes to this bug.