Bug 8398

Summary:

REGRESSION: LABEL in OPTION element is clobbering display #TEXT

Product:

WebKit

Reporter:

Bakafish <jason>

Component:

Forms

Assignee:

David Carson <dacarson>

Status:

RESOLVED FIXED

Severity:

Normal

CC:

alice.barraclough, ap, dacarson, darin, webkitBugzilla

Priority:

Keywords:

EasyFix, HasReduction, InRadar, Regression

Version:

420+

Hardware:

Mac

OS:

OS X 10.4

Attachments:

Description	Flags
test case	none
patch, no change log or test case yet	none
Patch	adele: review+
updated with new test cases	mjs: review+
only change to the patch was to remove tabs and replace with spaces	darin: review+

Bakafish

Reported 2006-04-14 14:29:15 PDT

This may be the desired behavior, it is not how Mozilla or current Safari implementations display it though. I would think that if there is a valid #TEXT child that that should take priority over the LABEL value. <html><head></head> <body> <form> Should say bar (I think.) <select> <option label="foo">bar</option> </select> </form> </body> </html>

Attachments
test case (150 bytes, text/html) 2006-04-15 02:44 PDT, Alexey Proskuryakov	no flags	Details
patch, no change log or test case yet (1.40 KB, patch) 2006-04-16 23:53 PDT, Darin Adler	no flags	Details Formatted Diff Diff
Patch (13.44 KB, patch) 2006-05-22 16:44 PDT, David Carson	adele: review+	Details Formatted Diff Diff
updated with new test cases (17.68 KB, patch) 2006-05-22 20:40 PDT, David Carson	mjs: review+	Details Formatted Diff Diff
only change to the patch was to remove tabs and replace with spaces (17.69 KB, patch) 2006-05-31 09:58 PDT, David Carson	darin: review+	Details Formatted Diff Diff
Show Obsolete (3) View All Add attachment proposed patch, testcase, etc.

Alexey Proskuryakov

Comment 1 2006-04-15 02:44:15 PDT

Confirmed; WinIE 6 also has the same behavior as Firefox and shipping Safari. BTW, attaching test cases to the bugs (rather than pasting inline) makes them somewhat easier to work on.

Alexey Proskuryakov

Comment 2 2006-04-15 02:44:55 PDT

Created attachment 7725 [details] test case Same test case, as an attachment.

Darin Adler

Comment 3 2006-04-16 23:52:56 PDT

The regression was caused when we fixed bug 7255. That bug complained that the label was ignored altogether. We need to change RenderSelect::updateFromElement to use the label only when the text is empty.

Darin Adler

Comment 4 2006-04-16 23:53:22 PDT

Created attachment 7756 [details] patch, no change log or test case yet

mitz

Comment 5 2006-04-18 10:11:31 PDT

I think the fix for this was accidently committed in r13955 together with the fix for bug 8437. Leaving the bug open for now since at least the change log still needs to be updated.

Darin Adler

Comment 6 2006-04-18 11:31:11 PDT

Oops!

Darin Adler

Comment 7 2006-04-18 11:37:42 PDT

I'll need to fix the change log and write a layout test.

Darin Adler

Comment 8 2006-04-18 17:09:22 PDT

I'm going to roll back my fix. The tests fast/forms/HTMLOptionElement_label0[1-5] make it clear that we researched this when we implemented this. Alexey says that IE 6 has the same behavior as Firefox, but these tests seem to indicate otherwise. More research is required.

Darin Adler

Comment 9 2006-04-18 17:40:13 PDT

I just tested with WinIE and my results match Alexey's. Those HTMLOptionElement tests are wrong. I have to fix them.

Darin Adler

Comment 10 2006-04-20 23:54:17 PDT

The good news is, we don't have to make new test cases -- just fix those old ones. But anyone can do this based on my patch, so I'm throwing this back to "unassigned". Be sure to test what happens when the element does have content, but it's all whitespace, or if the content has something other than plain text in it.

Alexey Proskuryakov

Comment 11 2006-04-26 21:52:12 PDT

*** Bug 8617 has been marked as a duplicate of this bug. ***

David Carson

Comment 12 2006-05-19 16:09:03 PDT

I'm confused on what is the correct behaviour. I tested the test cases fast/forms/HTMLOptionElement_label0[1-5] in Safari and in FF, and they both render them the same. When I tested them in IE7 beta2, the handling is different. But according to the comment #8 IE and FF behave the same. This is not what I am seeing. I am testing with TOT Safari and FF 1.5.0.3

Alexey Proskuryakov

Comment 13 2006-05-20 02:17:40 PDT

I have just re-tested with: Safari (Mac OS X 10.4.6) Mac Firefox 1.5.0.1 WinIE 6.0.2900.2180.xpsp_sp2_rtm.040803-2158 ToT WebKit r14501 Stock Safari, Firefox and WinIE gave the same results; ToT was different in each test. I don't have IE7.

David Carson

Comment 14 2006-05-21 14:33:58 PDT

Retested. Got the same results as Alexey. However, Win IE7beta7 does the same as TOT Safari. I wonder if IE are changing their behaviour. Which result is correct, IE6, Firefox & stock Safari or WinIE7 & TOT Safari?

Alice Liu

Comment 15 2006-05-22 00:25:39 PDT

<rdar://problem/4556767>

David Carson

Comment 16 2006-05-22 16:44:03 PDT

Created attachment 8468 [details] Patch Applied patch to new code base - original patch file has moved. Included ChangeLogs, updated test cases and 2 new test cases.

Adele Peterson

Comment 17 2006-05-22 16:47:57 PDT

Comment on attachment 8468 [details] Patch r=me

David Carson

Comment 18 2006-05-22 20:40:38 PDT

Created attachment 8470 [details] updated with new test cases Only change from previous patch was adding the new test cases.

Maciej Stachowiak

Comment 19 2006-05-22 20:45:00 PDT

Comment on attachment 8470 [details] updated with new test cases r=me

David Carson

Comment 20 2006-05-31 09:58:05 PDT

Created attachment 8626 [details] only change to the patch was to remove tabs and replace with spaces Remove tabs and repace with spaces. Fixed alignment of braces, ie {, in test case. No other changes.

Darin Adler

Comment 21 2006-05-31 11:02:05 PDT

Comment on attachment 8626 [details] only change to the patch was to remove tabs and replace with spaces r=me again

Bakafish

Comment 22 2006-05-31 15:50:44 PDT

Okay, after looking at the HTML 4.01 spec it seems that is actually behaving correctly. LABLE has priority over the #TEXT content. However, in quirks mode this has not been the behavior as far as I could tell. In quirks mode Safari 2.0.3 uses the #TEXT contents. Firefox uses the #TEXT in either case.

Darin Adler

Comment 23 2006-06-01 22:50:33 PDT

Committed revision 14679.

Note You need to log in before you can comment on or make changes to this bug.