Closed Bug 297537 Opened 19 years ago Closed 19 years ago

Gmail "rich text" composer buttons appear cut off

Categories

(Core :: Layout, defect)

defect
Not set
normal

Tracking

()

RESOLVED FIXED

People

(Reporter: uriber, Assigned: roc)

References

()

Details

(Keywords: regression, testcase)

Attachments

(5 files)

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.
Attached image screenshot
Attached file testcase
Minimal testcase. Notice that the problem is only in Quirks Mode.
Keywords: testcase
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
Flags: blocking1.8b3?
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.
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. 
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.
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
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.
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?
Blocks: 135994
No longer depends on: 135994
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.
... and then I'll have to fix this bug for real.
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.
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.
Attached patch fixSplinter Review
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+
Comment on attachment 186530 [details] [diff] [review]
fix

Fixes a gmail layout issue. Pretty low risk.
Attachment #186530 - Flags: approval1.8b3?
Attachment #186530 - Flags: approval1.8b3? → approval1.8b3+
Checked in.
Status: ASSIGNED → RESOLVED
Closed: 19 years ago
Resolution: --- → FIXED
Flags: blocking1.8b3?
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: