Bug 91954 – [Filesystem] Assertion on $0.webkitEntries while on the input field.

• Status: RESOLVED
• Resolution: FIXED
• Component: WebCore Misc.

= Step for Re-Produce =
1. Enter data:text/html,<input> into Omnibox
2. Hit [Enter] to display input element
3. Invoke Web Inspector
4. Select "Elements" view
5. Select "Properties"
6. Select "HTMLInputElement" or "Element" to display properties

= Results =
Got assertion failure with following message:
SHOULD NEVER BE REACHED
../../third_party/WebKit/Source/WebCore/html/InputType.cpp(698) : virtual WTF::String WebCore::InputType::droppedFileSystemId()

[Pavel Feldman]

Select input field in the inspector. In the console, type "$0.webkitEntries". See assertion:

	WebCore::InputType::droppedFileSystemId() [0x7f59d99ea539]
	WebCore::HTMLInputElement::droppedFileSystemId() [0x7f59d99b8ee4]
	WebCore::HTMLInputElementFileSystem::webkitEntries() [0x7f59db603e64]
	WebCore::HTMLInputElementV8Internal::webkitEntriesAttrGetter() [0x7f59da85b1b6]
	v8::internal::JSObject::GetPropertyWithCallback() [0x7f59daf00ea9]
	v8::internal::LoadIC::Load() [0x7f59db086c82]
	v8::internal::LoadIC_Miss() [0x7f59db0873ea]
	0x8c9b1f0618e

[Kinuko Yasuda]

Created attachment 154150 [details]
Patch

Comment on attachment 154150 [details]
Patch

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

> LayoutTests/ChangeLog:10
> +        * fast/forms/file/input-accessing-entries-expected.txt: Added.

Could you name test file as file-accessing-entries.html?
We're now arranging test case file name as ${inputType}-*.html.
Thanks for your cooperation!

> LayoutTests/fast/forms/file/input-accessing-entries.html:5
> +<pre id="console"></pre>

You don't need to put "console" markup by yourself.
js-test-pre.js creates it for you.
http://trac.webkit.org/browser/trunk/LayoutTests/fast/js/resources/js-test-pre.js

> LayoutTests/fast/forms/file/input-accessing-entries.html:13
> +var successfullyParsed = true;

You may not need to set successfullyParse by yourself.
Function  isSuccessfullyParsed() sets it to true for your.
http://trac.webkit.org/browser/trunk/LayoutTests/fast/js/resources/js-test-pre.js#L463

[Kinuko Yasuda]

Created attachment 154204 [details]
Patch

[Kinuko Yasuda]

Created attachment 154207 [details]
Patch

[Kinuko Yasuda]

Comment on attachment 154150 [details]
Patch

Thanks for your review!

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

>> LayoutTests/ChangeLog:10
>> +        * fast/forms/file/input-accessing-entries-expected.txt: Added.
> 
> Could you name test file as file-accessing-entries.html?
> We're now arranging test case file name as ${inputType}-*.html.
> Thanks for your cooperation!

Done.

>> LayoutTests/fast/forms/file/input-accessing-entries.html:5
>> +<pre id="console"></pre>
> 
> You don't need to put "console" markup by yourself.
> js-test-pre.js creates it for you.
> http://trac.webkit.org/browser/trunk/LayoutTests/fast/js/resources/js-test-pre.js

Oh I didn't know that... done.

>> LayoutTests/fast/forms/file/input-accessing-entries.html:13
>> +var successfullyParsed = true;
> 
> You may not need to set successfullyParse by yourself.
> Function  isSuccessfullyParsed() sets it to true for your.
> http://trac.webkit.org/browser/trunk/LayoutTests/fast/js/resources/js-test-pre.js#L463

Done.

[Kinuko Yasuda]

Could reviewers take a look?  Thanks!

[Kent Tamura]

Comment on attachment 154207 [details]
Patch

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

The code change looks ok.

> LayoutTests/ChangeLog:11
> +        * fast/forms/file/file-access-entries-expected.txt: Added.
> +        * fast/forms/file/file-access-entries.html: Added.

This test doesn't work on platforms without ENABLE_FILE_SYSTEM. So I think the test should be put into fast/filesystem/.

[Kinuko Yasuda]

Created attachment 155730 [details]
Patch

[Kinuko Yasuda]

Comment on attachment 154207 [details]
Patch

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

>> LayoutTests/ChangeLog:11
>> +        * fast/forms/file/file-access-entries.html: Added.
> 
> This test doesn't work on platforms without ENABLE_FILE_SYSTEM. So I think the test should be put into fast/filesystem/.

Ok... I was a bit separated since it's also a form test too but I moved it under fast/filesystem.
We have one more .webkitEntries test in fast/forms/file.  Should we move it (in another change) too then?

[Kent Tamura]

Comment on attachment 155730 [details]
Patch

ok

[WebKit Review Bot]

Comment on attachment 155730 [details]
Patch

Clearing flags on attachment: 155730

Committed r124309: <http://trac.webkit.org/changeset/124309>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.