Closed
Bug 297537
Opened 19 years ago
Closed 19 years ago
Gmail "rich text" composer buttons appear cut off
Categories
(Core :: Layout, defect)
Core
Layout
Tracking
()
RESOLVED
FIXED
People
(Reporter: uriber, Assigned: roc)
References
()
Details
(Keywords: regression, testcase)
Attachments
(5 files)
35.54 KB,
image/png
|
Details | |
463 bytes,
text/html
|
Details | |
517 bytes,
application/xhtml+xml
|
Details | |
361 bytes,
text/html
|
Details | |
7.49 KB,
patch
|
dbaron
:
review+
dbaron
:
superreview+
asa
:
approval1.8b3+
|
Details | Diff | Splinter Review |
On the Gmail compose page, the "rich text" buttons appear below where they should, and are mostly obscured by the main textbox. Mozilla/5.0 (Macintosh; U; PPC Mac OS X Mach-O; en-US; rv:1.8b2) Gecko/20050611 Firefox/1.0+ This WFM on Gecko 1.7 => regression. Screenshot and minimal testcase coming up.
Reporter | ||
Comment 1•19 years ago
|
||
Reporter | ||
Comment 2•19 years ago
|
||
Minimal testcase. Notice that the problem is only in Quirks Mode.
Comment 3•19 years ago
|
||
I don't see the bug with the url, but I can see the bug with the testcase. This regressed between 2005-03-06 and 2005-03-07. I've applied the patch from bug 276602 in a build that didn't have this bug and afterwards I did have this bug.
Blocks: 276602
Comment 4•19 years ago
|
||
Updated•19 years ago
|
Flags: blocking1.8b3?
Assignee | ||
Comment 5•19 years ago
|
||
We build a *very* strange frame tree in this testcase. Here's a teaser: Block(div)(1)@0x1a08fb8 next=0x1a205d0 {0,0,8910,90} [state=00000018] [overflow=0,0,8910,360] sc=0x1a08f68(i=1,b=0)< line 0x1a20590: count=5 state=inline,clean,prevmarginclean,not impacted,not wrapped,before:nobr,after:nobr[0x14100] {15,15,4455,60} ca={15,15,4455,345} < Text(0)@0x1a090e8[0,9,T] next=0x1a1e370 {15,75,0,0} [state=51100020] sc=0x1a09050 pst=:-moz-non-element< "\n " > Placeholder(table)(1)@0x1a1e370 {15,75,0,0} outOfFlowFrame=TableOuter(table)(1)@0x1a1dfd8 Text(2)@0x1a1f418[0,10,T] next=0x1a1f5f8 {15,75,0,0} [state=51100020] sc=0x1a09050 pst=:-moz-non-element< "\n\n " > Inline(table)(3)@0x1a1f5f8 next=0x1a20530 {15,15,4455,60} [state=00000008] [content=0xef6ba0] [overflow=0,0,4455,345] [sc=0x1a1f4c8]< Text(0)@0x1a1f6f0[0,13,T] next=0x1a1f920 {0,60,0,0} [state=51100020] sc=0x1a1f658 pst=:-moz-non-element< "\n " > TableOuter(table)(3)@0x1a1f920 {0,0,4455,60} [state=00010008] [content=0xef6ba0] [overflow=0,0,4455,345] [sc=0x1a1f9a0] pst=:-moz-table-outer< Table(table)(3)@0x1a1fb80 {0,0,4455,60} [state=00010008] [content=0xef6ba0] [overflow=0,0,4455,345] [sc=0x1a1f888] pst=:-moz-table< TableRowGroup(tbody)(1)@0x1a1fc88 {30,30,4395,0} [state=00010008] [content=0x1a00820] [overflow=0,0,4395,315] [sc=0x1a1f750]< Note that the table has ended up as a child of an inline. That's very wrong. We should have triggered an IB reconstruction and put the table on its own line.
Assignee | ||
Comment 6•19 years ago
|
||
We actually build this for both testcases, but somehow this actually works OK in standards mode. I think what should be happening in both modes is equivalent to replacing <table style="display:inline"> with <span>: it becomes an inline element, and we ignore the <td>s and <tr>s inside it.
Oh, I have seen this frame tree very often before it is like this always when display:inline on a table is specified. I thought thats OK.
Assignee | ||
Comment 8•19 years ago
|
||
Okay, what's happening here is that we create an inline frame for the <table> (good), then we create anonymous tableouter/table frames for the <tbody> that the parser generated (okay), and we treat those frames as if they were inline-table (which we don't support); this violates the CSS 2.1 spec which clearly says anonymous table boxes should be 'table' (not 'inline-table'). So there's one bug, although we might need to stick to our existing behaviour in quirks mode, because according to bernd this is intentional on our part. If we accept that we need limited inline-table support here, then we need to fix this bug with the frame tree we've got. I'll look at that tomorrow.
Reporter | ||
Comment 9•19 years ago
|
||
BTW, if you want to see the bug on Gmail on Windows, use ctrl-minus to reduce the font size. The bug only appears if the floated (first) table is small enough - which happens to be true on Mac with the default font size, but not on Windows. Thanks to Jomel who figured this out. Changing to All/All.
OS: MacOS X → All
Hardware: Macintosh → All
Comment 10•19 years ago
|
||
It's "intentional" as in "when IB splits were implemented, waterson probably didn't catch the fact that table frame construction uses none of the standard frame construction codepaths". I believe that the fact that we don't create an IB split here is in fact bug 135994 and we need to fix it.
Assignee | ||
Comment 11•19 years ago
|
||
I have a fix for bug 135994. Gmail doesn't work with the fix. There are five sets of buttons; each set is given as a table with display:inline, so each set of buttons appears on its own line. I believe this is correct as per the CSS spec. So what to do? Do we land the fix and get GMail and possibly other sites to fix themselves? Or do we do some quirk thing?
Assignee | ||
Comment 12•19 years ago
|
||
http://weblogs.mozillazine.org/roc/archives/2005/06/displayinline_a.html
Assignee | ||
Updated•19 years ago
|
Assignee | ||
Comment 13•19 years ago
|
||
I'm convinced that we need to keep supporting this. Basically the spec should be amended so that the anonymous table frame becomes an inline-table. If the spec cannot be so amended, then we'll have to make it a quirk. I will hook up nsCSSFrameConstructor to support inline-table and make the anonymous table frame an inline-table, probably by introducing a new nsCSSAnonBoxes psuedoelement.
Assignee | ||
Comment 14•19 years ago
|
||
... and then I'll have to fix this bug for real.
Assignee | ||
Comment 15•19 years ago
|
||
This is a further simplified testcase that fails in standards mode. I've made it more clear what's really going on here. The bug is that nsLineLayout sets the availableHeight of the frames it reflows to be the height of the "band" that it's currently using. This band is a rectangle representing the currently available space for laying out text between floats. In this testcase the first band is only 1 pixel high. The table frame sees that its height is constrained to one pixel and tries to split itself as if there's a page break there, but of course that doesn't work right because we're not really in a constrained height situation. nsLineLayout should probably just set the availableHeight to unconstrained, always. If the line doesn't fit on the page it will be moved to the next page. If we start getting into situations when inline-blocks and inline-tables need to split across pages, we can deal with that then. It's not immediately obvious how to handle that.
Assignee | ||
Comment 16•19 years ago
|
||
I believe that currently no genuine inline frames actually use the available height for anything, otherwise we'd have seen problems here before. Also note that although this is a regression from bug 276602, that's only because 276602 made zero-width floats start to affect the band setup (which is correct). I think attachment 186528 [details] would have failed on builds before that.
Setting available height based on bands is definitely broken.
Assignee | ||
Comment 18•19 years ago
|
||
This fixes the testcases, and gmail too.
Assignee: nobody → roc
Status: NEW → ASSIGNED
Attachment #186530 -
Flags: superreview?(dbaron)
Attachment #186530 -
Flags: review?(dbaron)
Attachment #186530 -
Flags: superreview?(dbaron)
Attachment #186530 -
Flags: superreview+
Attachment #186530 -
Flags: review?(dbaron)
Attachment #186530 -
Flags: review+
Assignee | ||
Comment 19•19 years ago
|
||
Comment on attachment 186530 [details] [diff] [review] fix Fixes a gmail layout issue. Pretty low risk.
Attachment #186530 -
Flags: approval1.8b3?
Updated•19 years ago
|
Attachment #186530 -
Flags: approval1.8b3? → approval1.8b3+
Assignee | ||
Comment 20•19 years ago
|
||
Checked in.
Status: ASSIGNED → RESOLVED
Closed: 19 years ago
Resolution: --- → FIXED
Updated•19 years ago
|
Flags: blocking1.8b3?
You need to log in
before you can comment on or make changes to this bug.
Description
•