Bug 65008 - REGRESSION(r89535): Form reset doesn't redraw a file upload control
Summary: REGRESSION(r89535): Form reset doesn't redraw a file upload control
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Forms (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Nobody
URL: data:text/html,<form><input type=file...
Keywords:
Depends on:
Blocks:
 
Reported: 2011-07-21 21:18 PDT by Kent Tamura
Modified: 2011-09-11 23:05 PDT (History)
2 users (show)

See Also:


Attachments
Patch (5.40 KB, patch)
2011-07-22 04:46 PDT, Kentaro Hara
no flags Details | Formatted Diff | Diff
Patch (5.45 KB, patch)
2011-07-22 05:36 PDT, Kentaro Hara
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Kent Tamura 2011-07-21 21:18:46 PDT
1. Open data:text/html,<form><input type=file><input type=reset></form>
2. Select a file for the file upload control
3. Click the reset button

Expected: The file upload control is cleared and shows "No file chosen"

Actual: The file upload control doesn't change. If the file upload control is repainted (e.g. inspect the file upload control), it shows "No file chosen".
Comment 1 Kent Tamura 2011-07-21 21:19:31 PDT
Chrome 12 doesn't have this issue.
Comment 2 Kentaro Hara 2011-07-21 22:46:16 PDT
I confirmed on Mac Safari that this is a regression caused by r89535 (https://bugs.webkit.org/show_bug.cgi?id=62069). I am making a patch.
Comment 3 Kent Tamura 2011-07-21 22:49:00 PDT
Thank you for the investigation.
Comment 4 Kentaro Hara 2011-07-22 04:46:58 PDT
Created attachment 101711 [details]
Patch
Comment 5 Kent Tamura 2011-07-22 04:56:01 PDT
Comment on attachment 101711 [details]
Patch

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

The code change looks ok.

> LayoutTests/fast/forms/file-input-reset-expected.html:16
> +<div id="console">
> +<p>
> +This tests whether the label next to the file chooser button becomes "No file chosen"
> +when we press the reset button.
> +To run this test manually, choose a file and then click the reset button.
> +If the label next to the file chooser button changes to "No file chosen",the test passes.
> +</p>

<div id=console> is not needed.

> LayoutTests/fast/forms/file-input-reset.html:10
> +<div id="console">

ditto.

> LayoutTests/fast/forms/file-input-reset.html:24
> +    var reset = document.getElementById("reset");
> +    click(reset.offsetLeft + reset.offsetWidth / 2, reset.offsetTop + reset.offsetHeight / 2);
> +    click(reset.offsetLeft + reset.offsetWidth + 10, reset.offsetTop + reset.offsetHeight + 10); // Move a cursor out of the reset button.

Why don't you use getElementById('form').reset()?

> Source/WebCore/ChangeLog:8
> +        This is a regression caused by r98535. This patch fixes the code

r98535 -> r89535.
Anyway, If you change the 1-list summary to "REGRESSION(r89535): Form reset doesn't redraw a file upload control", this sentence is not needed.
Comment 6 Kent Tamura 2011-07-22 04:57:04 PDT
(In reply to comment #5)
> Anyway, If you change the 1-list summary to "REGRESSION(r89535): Form reset doesn't redraw a file upload control", this sentence is not needed.

1-list -> 1-line
Comment 7 Kentaro Hara 2011-07-22 05:36:23 PDT
Created attachment 101719 [details]
Patch
Comment 8 Kentaro Hara 2011-07-22 05:36:32 PDT
Comment on attachment 101711 [details]
Patch

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

>> LayoutTests/fast/forms/file-input-reset-expected.html:16
>> +</p>
> 
> <div id=console> is not needed.

I added a selected file name here, in order to confirm that the file is really selected before being reset.

>> LayoutTests/fast/forms/file-input-reset.html:10
>> +<div id="console">
> 
> ditto.

Ditto.

>> LayoutTests/fast/forms/file-input-reset.html:24
>> +    click(reset.offsetLeft + reset.offsetWidth + 10, reset.offsetTop + reset.offsetHeight + 10); // Move a cursor out of the reset button.
> 
> Why don't you use getElementById('form').reset()?

I found that getElementById('form').reset() does not work. Even updateFromElement() is not invoked. This seems to be a regression too. Thus, I would like to commit this patch as it is, and then investigate the problem of reset() in the next patch.

>> Source/WebCore/ChangeLog:8
>> +        This is a regression caused by r98535. This patch fixes the code
> 
> r98535 -> r89535.
> Anyway, If you change the 1-list summary to "REGRESSION(r89535): Form reset doesn't redraw a file upload control", this sentence is not needed.

Done.
Comment 9 Kent Tamura 2011-07-22 05:38:33 PDT
Comment on attachment 101719 [details]
Patch

Looks good.
Thank you for fixing this!
Comment 10 Kentaro Hara 2011-07-22 06:29:09 PDT
Comment on attachment 101711 [details]
Patch

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

>>> LayoutTests/fast/forms/file-input-reset.html:24
>>> +    click(reset.offsetLeft + reset.offsetWidth + 10, reset.offsetTop + reset.offsetHeight + 10); // Move a cursor out of the reset button.
>> 
>> Why don't you use getElementById('form').reset()?
> 
> I found that getElementById('form').reset() does not work. Even updateFromElement() is not invoked. This seems to be a regression too. Thus, I would like to commit this patch as it is, and then investigate the problem of reset() in the next patch.

Sorry, this is not a bug. When we write like this

<form id="form">
<input id="reset" type="reset" />
</form>

then getElementById("form").reset is overridden by the reset input element, and thus getElementById("form").reset() does not work as expected.
Comment 11 WebKit Review Bot 2011-07-22 06:41:12 PDT
Comment on attachment 101719 [details]
Patch

Clearing flags on attachment: 101719

Committed r91566: <http://trac.webkit.org/changeset/91566>