WebKit Bugzilla
New
Browse
Log In
×
Sign in with GitHub
or
Remember my login
Create Account
·
Forgot Password
Forgotten password account recovery
VERIFIED FIXED
5656
REGRESSION: Buttons on Yahoo! Mail misplaced in ToT
https://bugs.webkit.org/show_bug.cgi?id=5656
Summary
REGRESSION: Buttons on Yahoo! Mail misplaced in ToT
Samuel Allen
Reported
2005-11-07 15:51:58 PST
In the latest builds the new button code is enabled. This works great on most sites, but the buttons at the top of Yahoo! Mail are misaligned. They appear too low. I think the problem is obvious when you look at the screenshot.
Attachments
Screenshot of top of Yahoo! Mail
(57.70 KB, image/tiff)
2005-11-07 15:52 PST
,
Samuel Allen
no flags
Details
Firefox screenshot (correct rendering)
(75.12 KB, image/tiff)
2005-11-07 15:59 PST
,
Samuel Allen
no flags
Details
A reduced testcase for Yahoo! Mail bug
(421 bytes, text/html)
2005-11-17 15:26 PST
,
Samuel Allen
no flags
Details
Minimized testcase
(167 bytes, text/html)
2005-11-17 22:46 PST
,
Samuel Allen
no flags
Details
Patch v1
(6.10 KB, patch)
2006-01-19 22:46 PST
,
David Kilzer (:ddkilzer)
mjs
: review-
Details
Formatted Diff
Diff
Image v1
(10.46 KB, image/png)
2006-01-19 22:48 PST
,
David Kilzer (:ddkilzer)
no flags
Details
Patch v2
(7.90 KB, patch)
2006-01-22 00:39 PST
,
David Kilzer (:ddkilzer)
darin
: review-
Details
Formatted Diff
Diff
Test Image Expected v2
(29.43 KB, image/png)
2006-01-22 00:47 PST
,
David Kilzer (:ddkilzer)
no flags
Details
Proposed layout test
(584 bytes, text/html)
2006-01-22 12:49 PST
,
David Kilzer (:ddkilzer)
no flags
Details
Patch v3
(9.02 KB, patch)
2006-01-22 15:47 PST
,
David Kilzer (:ddkilzer)
no flags
Details
Formatted Diff
Diff
Test Image Expected v3
(43.93 KB, image/png)
2006-01-22 15:48 PST
,
David Kilzer (:ddkilzer)
no flags
Details
Patch v4
(8.73 KB, patch)
2006-01-22 16:12 PST
,
David Kilzer (:ddkilzer)
mjs
: review+
Details
Formatted Diff
Diff
Test Image Expected v4
(36.45 KB, image/png)
2006-01-22 16:13 PST
,
David Kilzer (:ddkilzer)
no flags
Details
Show Obsolete
(7)
View All
Add attachment
proposed patch, testcase, etc.
Samuel Allen
Comment 1
2005-11-07 15:52:31 PST
Created
attachment 4623
[details]
Screenshot of top of Yahoo! Mail
Dave Hyatt
Comment 2
2005-11-07 15:54:55 PST
What do they look like in Firefox?
Samuel Allen
Comment 3
2005-11-07 15:59:54 PST
Created
attachment 4624
[details]
Firefox screenshot (correct rendering)
Dave Hyatt
Comment 4
2005-11-08 13:42:27 PST
I'm not sure if this is a button problem or a white-space problem. The buttons in the rightmost cell are wrapping now (despite the cell having a nowrap specified). This needs a reduction.
Samuel Allen
Comment 5
2005-11-17 15:23:44 PST
Ok I did some digging into this problem and created a reduced testcase. To my surprise, I don't think buttons are to blame. Basically there are two buttons in a table cell with nowrap specified. The ID of one of them is "searchmail" and the ID of the other is "searchtheweb". However, the stylesheet associated with the page only contains styles for "searchmail1" and "searchtheweb". When the 1 is taken off, all three browsers work. When the 1 remains, Safari 2.0.2, Firefox 1.5 succeed but WebKit ToT fails. Has something changed in selector parsing or matching in the recently? Should "searchmail1" match "searchmail"? I don't know the answers to these questions. I will, however attach the reduced testcase. It consists of two buttons, which in all browsers other than ToT line up on one line.
Samuel Allen
Comment 6
2005-11-17 15:26:21 PST
Created
attachment 4717
[details]
A reduced testcase for Yahoo! Mail bug This works on all browsers except ToT.
Samuel Allen
Comment 7
2005-11-17 15:30:45 PST
Actually, the "searchmail1" rule is not applied in any of the testcases. So I don't have a clue what the problem is. As I said, the reduced testcase works on every browser except ToT.
Samuel Allen
Comment 8
2005-11-17 22:44:37 PST
The problem is with "nowrap". The test case features a td cell with nowrap specified, and then two buttons, the second of which has style="white-space:nowrap". When either nowrap is removed, the testcase lays out correctly.
Samuel Allen
Comment 9
2005-11-17 22:46:40 PST
Created
attachment 4720
[details]
Minimized testcase Works correctly in Firefox and Safari, not on ToT.
Eric Seidel (no email)
Comment 10
2005-12-28 01:55:14 PST
The test case looks like ot works correclty. Or?
Joost de Valk (AlthA)
Comment 11
2005-12-28 07:12:27 PST
No it doesn't, buttons should be next to each other. Reopening.
Eric Seidel (no email)
Comment 12
2005-12-28 13:23:22 PST
They appear next to each other to me [search mail] [search the web] ?
Joost de Valk (AlthA)
Comment 13
2005-12-29 00:16:11 PST
They don't to me, and this is latest ToT...
David Kilzer (:ddkilzer)
Comment 14
2006-01-08 12:48:32 PST
The space between the last </button> tag and the </td> tag is what causes the button to wrap. If you remove the space between these two tags, the HTML renders correctly: </button></td> ANY whitespace (spaces, tabs, newlines) will cause it to render incorrectly. Hopefully this will narrow down the cause!
Alice Liu
Comment 15
2006-01-10 10:58:53 PST
<
rdar://problem/4404335
>
David Kilzer (:ddkilzer)
Comment 16
2006-01-19 19:28:41 PST
I did a binary search through the nightly builds (thanks Mark Rowe!) and found where the bug was introduced: Testcase passes: WebKit-CVS-2005-10-22 04-42-12 GMT.dmg Testcase fails: WebKit-CVS-2005-10-23 02-27-01 GMT.dmg I will begin looking at commits during that time to see if I can determine which change caused this bug, unless someone beats me to it!
David Kilzer (:ddkilzer)
Comment 17
2006-01-19 19:55:57 PST
Using "svn log -r{YYYY-MM-DD}", I determined that these CVS builds match the following Subversion revisions: Passed: WebKit-CVS-2005-10-22 04-42-12 GMT.dmg:
r10904
Failed: WebKit-CVS-2005-10-23 02-27-01 GMT.dmg:
r10916
David Kilzer (:ddkilzer)
Comment 18
2006-01-19 22:46:23 PST
Created
attachment 5792
[details]
Patch v1 The regression occurred in Subversion revision
r10909
. The fix is to change the default white-space attribute for the <button> tag in html4.css from 'normal' to 'pre' to match the other button-like items in the stylesheet. Included in the patch is a fix to a misleading comment (which doesn't match the code). One layout test also changed slightly: fast/forms/button-in-forms-collection. Taking this bug.
David Kilzer (:ddkilzer)
Comment 19
2006-01-19 22:48:53 PST
Created
attachment 5793
[details]
Image v1
David Kilzer (:ddkilzer)
Comment 20
2006-01-20 04:41:59 PST
Hrm...should probably mention the Radar bug number in the log message as well.
Maciej Stachowiak
Comment 21
2006-01-21 12:35:15 PST
I don't think this fix is correct. Buttons do not appear to have white-space: pre in other browsers. I tried this test case and both buttons looked the same in all browsers: <button>test button</button><br> <button> test button </button> However, buttons don't appear to force white-space: normal, instead they inherit the containing white-space mode of their container. For instance, in this case, the extra spaces do show up in Firefox, but not in ToT Safari: <pre> <button>test button</button><br> <button> test button </button> </pre> Therefore I think the right fix is to remove the white-space property for button entirely.
David Kilzer (:ddkilzer)
Comment 22
2006-01-22 00:22:26 PST
(In reply to
comment #21
)
> However, buttons don't appear to force white-space: normal, instead they > inherit the containing white-space mode of their container. For instance, in > this case, the extra spaces do show up in Firefox, but not in ToT Safari: > > <pre> > <button>test button</button><br> > <button> test button </button> > </pre>
Which version of Firefox are you using? I'm using 1.5 (Mozilla/5.0 (Macintosh; U; PPC Mac OS X Mach-O; en-US; rv:1.8) Gecko/20051111 Firefox/1.5), but the above two buttons look identical (no extra whitespace).
David Kilzer (:ddkilzer)
Comment 23
2006-01-22 00:39:51 PST
Created
attachment 5827
[details]
Patch v2 Updated patch per
Comment #21
, but see also
Comment #22
. Also added more HTML to test case per
Comment #21
.
David Kilzer (:ddkilzer)
Comment 24
2006-01-22 00:47:27 PST
Created
attachment 5828
[details]
Test Image Expected v2
David Kilzer (:ddkilzer)
Comment 25
2006-01-22 12:49:42 PST
Created
attachment 5846
[details]
Proposed layout test
Maciej Stachowiak
Comment 26
2006-01-22 14:34:58 PST
Comment on
attachment 5827
[details]
Patch v2 r=me Please add some more explanation of what the test case is testing however (for instance mention the bug number and that it is checking white-space mode for the <button> tag).
Darin Adler
Comment 27
2006-01-22 15:17:05 PST
Comment on
attachment 5827
[details]
Patch v2 Dave will do another round of revision on it the layout test and get reviewed again.
David Kilzer (:ddkilzer)
Comment 28
2006-01-22 15:47:48 PST
Created
attachment 5852
[details]
Patch v3 Updated layout test with more information per
Comment #26
. Updated ChangeLog entries. Revised *-expected.* files. New expected PNG soon.
David Kilzer (:ddkilzer)
Comment 29
2006-01-22 15:48:57 PST
Created
attachment 5853
[details]
Test Image Expected v3
David Kilzer (:ddkilzer)
Comment 30
2006-01-22 16:12:09 PST
Created
attachment 5855
[details]
Patch v4 Fixed HTML of layout test. Made sure expected image is correct, too.
David Kilzer (:ddkilzer)
Comment 31
2006-01-22 16:13:13 PST
Created
attachment 5856
[details]
Test Image Expected v4 Fixed expected image.
David Kilzer (:ddkilzer)
Comment 32
2006-01-22 20:26:04 PST
Verified this issue has been fixed by nightly build
r12295
(commit was made in
r12294
).
Eric Seidel (no email)
Comment 33
2006-01-31 21:20:50 PST
Removing Regression keyword from bugs already fixed.
Note
You need to
log in
before you can comment on or make changes to this bug.
Top of Page
Format For Printing
XML
Clone This Bug