Bug 91954 - [Filesystem] Assertion on $0.webkitEntries while on the input field.
Summary: [Filesystem] Assertion on $0.webkitEntries while on the input field.
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebCore Misc. (show other bugs)
Version: 528+ (Nightly build)
Hardware: All All
: P2 Normal
Assignee: Kinuko Yasuda
URL: data:text/html,<input>
Keywords:
Depends on:
Blocks:
 
Reported: 2012-07-22 20:32 PDT by yosin
Modified: 2012-08-01 00:57 PDT (History)
14 users (show)

See Also:


Attachments
Patch (4.19 KB, patch)
2012-07-24 15:17 PDT, Kinuko Yasuda
no flags Details | Formatted Diff | Diff
Patch (4.10 KB, patch)
2012-07-24 18:09 PDT, Kinuko Yasuda
no flags Details | Formatted Diff | Diff
Patch (4.09 KB, patch)
2012-07-24 18:27 PDT, Kinuko Yasuda
no flags Details | Formatted Diff | Diff
Patch (4.09 KB, patch)
2012-07-31 23:43 PDT, Kinuko Yasuda
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description yosin 2012-07-22 20:32:17 PDT
= 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()
Comment 1 Pavel Feldman 2012-07-23 06:16:41 PDT
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
Comment 2 Kinuko Yasuda 2012-07-24 15:17:32 PDT
Created attachment 154150 [details]
Patch
Comment 3 yosin 2012-07-24 17:59:32 PDT
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
Comment 4 Kinuko Yasuda 2012-07-24 18:09:06 PDT
Created attachment 154204 [details]
Patch
Comment 5 Kinuko Yasuda 2012-07-24 18:27:06 PDT
Created attachment 154207 [details]
Patch
Comment 6 Kinuko Yasuda 2012-07-24 18:29:27 PDT
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.
Comment 7 Kinuko Yasuda 2012-07-31 16:40:42 PDT
Could reviewers take a look?  Thanks!
Comment 8 Kent Tamura 2012-07-31 19:37:08 PDT
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/.
Comment 9 Kinuko Yasuda 2012-07-31 23:43:27 PDT
Created attachment 155730 [details]
Patch
Comment 10 Kinuko Yasuda 2012-07-31 23:56:27 PDT
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?
Comment 11 Kent Tamura 2012-07-31 23:58:02 PDT
Comment on attachment 155730 [details]
Patch

ok
Comment 12 WebKit Review Bot 2012-08-01 00:57:36 PDT
Comment on attachment 155730 [details]
Patch

Clearing flags on attachment: 155730

Committed r124309: <http://trac.webkit.org/changeset/124309>
Comment 13 WebKit Review Bot 2012-08-01 00:57:41 PDT
All reviewed patches have been landed.  Closing bug.