Summary: | fast/forms/min-content-form-controls.html fails on some platforms | ||||||
---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Ojan Vafai <ojan> | ||||
Component: | New Bugs | Assignee: | Ojan Vafai <ojan> | ||||
Status: | RESOLVED FIXED | ||||||
Severity: | Normal | CC: | eric, rniwa, tony | ||||
Priority: | P2 | ||||||
Version: | 528+ (Nightly build) | ||||||
Hardware: | Unspecified | ||||||
OS: | Unspecified | ||||||
Attachments: |
|
Description
Ojan Vafai
2013-01-09 12:14:13 PST
Created attachment 181965 [details]
Patch
Comment on attachment 181965 [details]
Patch
rs=me.
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 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 on attachment 181965 [details]
Patch
Oops, I apparently overrode tony’s cq-.
Committed r139221: <http://trac.webkit.org/changeset/139221> 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". (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. 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. |