Bug 106482 - fast/forms/min-content-form-controls.html fails on some platforms
Summary: fast/forms/min-content-form-controls.html fails on some platforms
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Ojan Vafai
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2013-01-09 12:14 PST by Ojan Vafai
Modified: 2013-01-09 15:02 PST (History)
3 users (show)

See Also:


Attachments
Patch (3.42 KB, patch)
2013-01-09 12:15 PST, Ojan Vafai
eric: review+
rniwa: commit-queue-
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Ojan Vafai 2013-01-09 12:14:13 PST
fast/forms/min-content-form-controls.html fails on some platforms
Comment 1 Ojan Vafai 2013-01-09 12:15:43 PST
Created attachment 181965 [details]
Patch
Comment 2 Eric Seidel (no email) 2013-01-09 12:19:27 PST
Comment on attachment 181965 [details]
Patch

rs=me.
Comment 3 Tony Chang 2013-01-09 12:19:41 PST
Comment on attachment 181965 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=181965&action=review

> LayoutTests/ChangeLog:10
> +        The text dump for some form controls (menulists?) is different
> +        on different platforms. Remove the form controls to avoid adding
> +        platform-specific expectations for this test.

How are they different?

> LayoutTests/fast/forms/min-content-form-controls.html:42
> +    // Remove these elements since some of the text dumps are different platforms.

What does "some of the text dumps are different platforms" mean?
Comment 4 Ryosuke Niwa 2013-01-09 12:20:16 PST
Comment on attachment 181965 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=181965&action=review

> LayoutTests/fast/forms/min-content-form-controls.html:47
> +function remove(id) {
> +    var node = document.getElementById(id);
> +    node.parentNode.removeChild(node);
> +}
> +
> +if (window.testRunner) {
> +    // Remove these elements since some of the text dumps are different platforms.
> +    ids.forEach(function(id) {
> +        remove(id + '-a');
> +        remove(id + '-b');
> +    });
> +}

Why don’t we just delete all input and select elements or simply wrap them in div and set innerHTML on that?
Comment 5 Ryosuke Niwa 2013-01-09 12:20:52 PST
Comment on attachment 181965 [details]
Patch

Oops, I apparently overrode tony’s cq-.
Comment 6 Ojan Vafai 2013-01-09 12:25:23 PST
Committed r139221: <http://trac.webkit.org/changeset/139221>
Comment 7 Darin Adler 2013-01-09 12:51:51 PST
Comment on attachment 181965 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=181965&action=review

>> LayoutTests/ChangeLog:10
>> +        The text dump for some form controls (menulists?) is different
>> +        on different platforms. Remove the form controls to avoid adding
>> +        platform-specific expectations for this test.
> 
> How are they different?

Why is the text dump for these form controls different on different platforms? This seems a fine workaround for that problem, but is it something we could easily fix in the form control code?

>> LayoutTests/fast/forms/min-content-form-controls.html:42
>> +    // Remove these elements since some of the text dumps are different platforms.
> 
> What does "some of the text dumps are different platforms" mean?

Missing words here. I think you mean "are different on different platforms".
Comment 8 Ojan Vafai 2013-01-09 14:46:25 PST
(In reply to comment #7)
> (From update of attachment 181965 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=181965&action=review
> 
> >> LayoutTests/ChangeLog:10
> >> +        The text dump for some form controls (menulists?) is different
> >> +        on different platforms. Remove the form controls to avoid adding
> >> +        platform-specific expectations for this test.
> > 
> > How are they different?
> 
> Why is the text dump for these form controls different on different platforms? This seems a fine workaround for that problem, but is it something we could easily fix in the form control code?

Yeah, sorry, I was being overly lazy. Looking closer, I was confused about what was going on. The issue is that there was line wrapping on some platforms due to differently sized form controls, which lead to a different number of spaces in the innerText output.

An alternate workaround for this test would have been to put a <br> after each form control.
Comment 9 Ojan Vafai 2013-01-09 14:47:32 PST
In fact, I like the explicit break solution better since we don't then have to do something different for being in the test runner. I'll commit a followup patch with that change. Thanks for calling me out on this.
Comment 10 Ojan Vafai 2013-01-09 15:02:18 PST
http://trac.webkit.org/changeset/139245