Bug 150909 - [Win] fast/forms/HTMLOptionElement_label03.html failing on win7
Summary: [Win] fast/forms/HTMLOptionElement_label03.html failing on win7
Alias: None
Product: WebKit
Classification: Unclassified
Component: Forms (show other bugs)
Version: WebKit Nightly Build
Hardware: PC Windows 7
: P2 Normal
Assignee: Keith Rollin
Keywords: InRadar
Depends on:
Reported: 2015-11-04 15:57 PST by Ryan Haddad
Modified: 2015-12-03 12:46 PST (History)
4 users (show)

See Also:

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

Note You need to log in before you can comment on or make changes to this bug.
Description Ryan Haddad 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>

Comment 1 Ryan Haddad 2015-11-04 16:03:02 PST
Marked as ImageOnlyFailure in <https://trac.webkit.org/r192041>
Comment 2 Keith Rollin 2015-11-05 09:31:20 PST
Chris, do you have a suggestion on how to address this? The test is comparing this:

        <option label=" ">white space label should display empty string to match IE</option>

To this:

        <option> </option>

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.
Comment 3 Radar WebKit Bug Importer 2015-11-13 13:28:06 PST
Comment 4 Keith Rollin 2015-11-13 15:09:58 PST
Created attachment 265502 [details]
Comment 5 Darin Adler 2015-11-14 18:48:08 PST
Comment on attachment 265502 [details]

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.
Comment 6 Keith Rollin 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?
Comment 7 Keith Rollin 2015-11-17 11:49:29 PST
Created attachment 265687 [details]
Comment 8 WebKit Commit Bot 2015-12-03 12:46:29 PST
Comment on attachment 265687 [details]

Clearing flags on attachment: 265687

Committed r193368: <http://trac.webkit.org/changeset/193368>
Comment 9 WebKit Commit Bot 2015-12-03 12:46:34 PST
All reviewed patches have been landed.  Closing bug.