Bug 150909

Summary:

[Win] fast/forms/HTMLOptionElement_label03.html failing on win7

Product:

WebKit

Reporter:

Ryan Haddad <ryanhaddad>

Component:

Forms

Assignee:

Keith Rollin <krollin>

Status:

RESOLVED FIXED

Severity:

Normal

CC:

cdumez, commit-queue, krollin, webkit-bug-importer

Priority:

Keywords:

InRadar

Version:

WebKit Nightly Build

Hardware:

OS:

Windows 7

Attachments:

Description	Flags
Patch	none
Patch	none

Ryan Haddad

Reported 2015-11-04 15:57:55 PST

[Win] fast/forms/HTMLOptionElement_label03.html failing on win7 after rebaseline in <http://trac.webkit.org/changeset/191964> <https://build.webkit.org/results/Apple%20Win%207%20Release%20(Tests)/r192034%20(54928)/results.html>

Attachments
Patch (5.89 KB, patch) 2015-11-13 15:09 PST, Keith Rollin	no flags	Details Formatted Diff Diff
Patch (5.54 KB, patch) 2015-11-17 11:49 PST, Keith Rollin	no flags	Details Formatted Diff Diff
Show Obsolete (1) View All Add attachment proposed patch, testcase, etc.

Ryan Haddad

Comment 1 2015-11-04 16:03:02 PST

Marked as ImageOnlyFailure in <https://trac.webkit.org/r192041>

Keith Rollin

Comment 2 2015-11-05 09:31:20 PST

Chris, do you have a suggestion on how to address this? The test is comparing this: <body> <select> <option label=" ">white space label should display empty string to match IE</option> </select> </body> To this: <body> <select> <option> </option> </select> </body> I expect that the DOM is stripping the spaces from the text content but not the label attribute. The tests probably match on some platforms because the platform itself strips spaces from menu labels. But I guess that doesn't happen on Win7. If we keep this test and this approach, it's not apparent to me how to make it pass.

Radar WebKit Bug Importer

Comment 3 2015-11-13 13:28:06 PST

<rdar://problem/23539673>

Keith Rollin

Comment 4 2015-11-13 15:09:58 PST

Created attachment 265502 [details] Patch

Darin Adler

Comment 5 2015-11-14 18:48:08 PST

Comment on attachment 265502 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=265502&action=review > Source/WebCore/html/HTMLOptionElement.cpp:271 > + return label.stripWhiteSpace(isHTMLSpace).simplifyWhiteSpace(isHTMLSpace); Stripping leading and trailing whitespace is correct. “Simplifying” whitespace seems highly unlikely to be correct.

Keith Rollin

Comment 6 2015-11-16 09:34:58 PST

Simplifying whitespace is what we were already doing to the text content when that was returned as the text to be displayed. I'm just applying that to the label value when that's being returned. That seems internally consistent, and matches what Chrome 46 and IE 11 are doing. But I guess you're saying that extra internal spaces in the label attribute are less likely to be incidental and are more likely to be intentional and so should be retained, regardless of the above?

Keith Rollin

Comment 7 2015-11-17 11:49:29 PST

Created attachment 265687 [details] Patch

WebKit Commit Bot

Comment 8 2015-12-03 12:46:29 PST

Comment on attachment 265687 [details] Patch Clearing flags on attachment: 265687 Committed r193368: <http://trac.webkit.org/changeset/193368>

WebKit Commit Bot

Comment 9 2015-12-03 12:46:34 PST

All reviewed patches have been landed. Closing bug.

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