Bug 10145 - REGRESSION: Allow line breaks between adjacent popups and buttons
Summary: REGRESSION: Allow line breaks between adjacent popups and buttons
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Forms (show other bugs)
Version: 420+
Hardware: Mac OS X 10.4
: P1 Normal
Assignee: Dave Hyatt
URL:
Keywords: InRadar, Regression
Depends on:
Blocks:
 
Reported: 2006-07-28 08:03 PDT by mitz
Modified: 2007-01-14 17:45 PST (History)
0 users

See Also:


Attachments
Test case (312 bytes, text/html)
2006-07-28 08:03 PDT, mitz
no flags Details
Test of other replaced elements (809 bytes, text/html)
2006-08-18 12:34 PDT, mitz
no flags Details
Test as rendered by WinIE (3.31 KB, image/png)
2006-08-18 12:35 PDT, mitz
no flags Details
Test as rendered by Firefox (4.59 KB, image/png)
2006-08-18 12:35 PDT, mitz
no flags Details
Patch that I think may be what we want (12.38 KB, patch)
2007-01-13 00:18 PST, Dave Hyatt
aroben: review+
Details | Formatted Diff | Diff
Really get every case. (10.82 KB, patch)
2007-01-14 17:18 PST, Dave Hyatt
oliver: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description mitz 2006-07-28 08:03:17 PDT
Lines should be allowed to break between adjacent <select> elements, even if there is no white space between them. The test case shows that WebKit no longer allows it. Firefox does.

I think the reason is that the native Mac popups are adjusted to white-space:pre.
Comment 1 mitz 2006-07-28 08:03:47 PDT
Created attachment 9745 [details]
Test case
Comment 2 mitz 2006-08-18 12:34:58 PDT
Created attachment 10129 [details]
Test of other replaced elements
Comment 3 mitz 2006-08-18 12:35:36 PDT
Created attachment 10130 [details]
Test as rendered by WinIE
Comment 4 mitz 2006-08-18 12:35:56 PDT
Created attachment 10131 [details]
Test as rendered by Firefox
Comment 5 mitz 2006-08-18 12:36:55 PDT
As the extended test shows, WebKit has the same problem with <input type="button">.
Comment 6 Stephanie Lewis 2006-11-06 19:43:26 PST
radar 4823049
Comment 7 Dave Hyatt 2007-01-13 00:18:14 PST
Created attachment 12411 [details]
Patch that I think may be what we want

I'm a little hesitant about this, but it does seem to match Firefox.  Basically we use the parent white-space when looking at replaced elements.  This seems to be what Firefox is doing.
Comment 8 Darin Adler 2007-01-13 07:27:11 PST
Comment on attachment 12411 [details]
Patch that I think may be what we want

You attached the wrong patch. This patch is removing the marquee unfurl style!
Comment 9 Dave Hyatt 2007-01-13 13:12:55 PST
It's not the wrong patch.  I had to remove the whiteSpace() flag from marquees since with this patch it's no longer needed.   While in there I decided to yank the unfurl thing, since it was just for Safari RSS prototyping and never used.

The important bit is in bidi.cpp near the bottom.

Comment 10 Dave Hyatt 2007-01-13 15:37:22 PST
Comment on attachment 12411 [details]
Patch that I think may be what we want

Setting review flag (timidly).
Comment 11 Adam Roben (:aroben) 2007-01-13 23:14:37 PST
Comment on attachment 12411 [details]
Patch that I think may be what we want

Turn the testcase into a layout test, and r=me.
Comment 12 Dave Hyatt 2007-01-14 02:00:27 PST
I landed this patch, but as Mitz pointed out, there's still the problem of replaced elements followed by non-replaced elements.
Comment 13 Dave Hyatt 2007-01-14 02:04:05 PST
NEver mind, it's ok.  I'm going to add an additional test.
Comment 14 mitz 2007-01-14 06:31:00 PST
This case is still misbehaving:

<div style="width: 5px"><select></select><select></select><select></select></div>
Comment 15 Dave Hyatt 2007-01-14 17:18:55 PST
Created attachment 12436 [details]
Really get every case.

Here we go.
Comment 16 Dave Hyatt 2007-01-14 17:45:48 PST
Fixed in r18851.