Bug 77930 - INPUT shouldn't create ShadowRoot dynamically.
: INPUT shouldn't create ShadowRoot dynamically.
Status: RESOLVED FIXED
: WebKit
HTML DOM
: 528+ (Nightly build)
: Unspecified Unspecified
: P2 Normal
Assigned To:
:
:
:
: 77503 77608
  Show dependency treegraph
 
Reported: 2012-02-06 20:31 PST by
Modified: 2012-05-30 18:55 PST (History)


Attachments
W.I.P. (15.40 KB, patch)
2012-02-07 23:27 PST, Shinya Kawanaka
no flags Review Patch | Details | Formatted Diff | Diff
W.I.P. (15.40 KB, patch)
2012-02-07 23:37 PST, Shinya Kawanaka
no flags Review Patch | Details | Formatted Diff | Diff
Patch (18.58 KB, patch)
2012-02-09 18:12 PST, Shinya Kawanaka
no flags Review Patch | Details | Formatted Diff | Diff
Patch (18.52 KB, patch)
2012-02-09 22:18 PST, Shinya Kawanaka
no flags Review Patch | Details | Formatted Diff | Diff
Patch (18.54 KB, patch)
2012-02-10 00:05 PST, Shinya Kawanaka
no flags Review Patch | Details | Formatted Diff | Diff


Note

You need to log in before you can comment on or make changes to this bug.


Description From 2012-02-06 20:31:43 PST
Currently INPUT recreates shadow root if its type is changed.
We can reuse shadow root to stop recreating it.
------- Comment #1 From 2012-02-07 23:27:08 PST -------
Created an attachment (id=126017) [details]
W.I.P.
------- Comment #2 From 2012-02-07 23:37:34 PST -------
Created an attachment (id=126018) [details]
Patch
------- Comment #3 From 2012-02-08 01:26:40 PST -------
(From update of attachment 126018 [details])
Attachment 126018 [details] did not pass chromium-ews (chromium-xvfb):
Output: http://queues.webkit.org/results/11460321

New failing tests:
fast/speech/speech-input-scripting.html
------- Comment #4 From 2012-02-09 18:12:32 PST -------
Created an attachment (id=126420) [details]
Patch
------- Comment #5 From 2012-02-09 21:05:52 PST -------
(From update of attachment 126420 [details])
View in context: https://bugs.webkit.org/attachment.cgi?id=126420&action=review

> Source/WebCore/ChangeLog:12
> +        Media control elements are implemented input elements. These elements should also

The meaning of "Media control elements are implemented input elements" is not clear to me.

> Source/WebCore/html/FileInputType.cpp:270
> +    element()->shadowRoot()->appendChild(element()->multiple() ? UploadButtonElement::createForMultiple(element()->document()): UploadButtonElement::create(element()->document()), ec);

Won’t this be problematic if I attach an author shadow root to the input element? Then the UploadButtonElement will be leaked into my author shadow root.

> Source/WebCore/html/InputType.cpp:382
> +        while (root->hasChildNodes())

What about root->removeAllChildren() or root->removeChildren()

*I’m not sure which one of these you want. Probably removeChildren.*
------- Comment #6 From 2012-02-09 21:53:44 PST -------
(From update of attachment 126420 [details])
View in context: https://bugs.webkit.org/attachment.cgi?id=126420&action=review

>> Source/WebCore/html/FileInputType.cpp:270
>> +    element()->shadowRoot()->appendChild(element()->multiple() ? UploadButtonElement::createForMultiple(element()->document()): UploadButtonElement::create(element()->document()), ec);
> 
> Won’t this be problematic if I attach an author shadow root to the input element? Then the UploadButtonElement will be leaked into my author shadow root.

Since we don't support multiple shadow subtrees, and we have disabled to add an author shadow root into INPUT element, this shouldn't cause a problem.
I'm now writing a patch to support multiple shadow subtrees. Your anxiety will be fixed in that (coming) patch.
------- Comment #7 From 2012-02-09 21:56:05 PST -------
(From update of attachment 126420 [details])
View in context: https://bugs.webkit.org/attachment.cgi?id=126420&action=review

>> Source/WebCore/html/InputType.cpp:382
>> +        while (root->hasChildNodes())
> 
> What about root->removeAllChildren() or root->removeChildren()
> 
> *I’m not sure which one of these you want. Probably removeChildren.*

Ah, I didn't notice ContainerNode has removeChildren() method...
------- Comment #8 From 2012-02-09 22:18:21 PST -------
Created an attachment (id=126453) [details]
Patch
------- Comment #9 From 2012-02-09 22:49:53 PST -------
(From update of attachment 126453 [details])
Attachment 126453 [details] did not pass chromium-ews (chromium-xvfb):
Output: http://queues.webkit.org/results/11447573

New failing tests:
fast/speech/speech-input-scripting.html
------- Comment #10 From 2012-02-09 23:15:10 PST -------
(In reply to comment #9)
> (From update of attachment 126453 [details] [details])
> Attachment 126453 [details] [details] did not pass chromium-ews (chromium-xvfb):
> Output: http://queues.webkit.org/results/11447573
> 
> New failing tests:
> fast/speech/speech-input-scripting.html

Why is chromium bot complaining...?
------- Comment #11 From 2012-02-10 00:01:19 PST -------
(In reply to comment #10)
> (In reply to comment #9)
> > (From update of attachment 126453 [details] [details] [details])
> > Attachment 126453 [details] [details] [details] did not pass chromium-ews (chromium-xvfb):
> > Output: http://queues.webkit.org/results/11447573
> > 
> > New failing tests:
> > fast/speech/speech-input-scripting.html
> 
> Why is chromium bot complaining...?

Ah, removeChildren() is not correct, maybe.
------- Comment #12 From 2012-02-10 00:05:56 PST -------
Created an attachment (id=126468) [details]
Patch
------- Comment #13 From 2012-02-10 09:19:08 PST -------
(From update of attachment 126468 [details])
This is great! Nice and simple.
------- Comment #14 From 2012-02-12 19:53:22 PST -------
(From update of attachment 126468 [details])
Clearing flags on attachment: 126468

Committed r107524: <http://trac.webkit.org/changeset/107524>
------- Comment #15 From 2012-02-12 19:53:28 PST -------
All reviewed patches have been landed.  Closing bug.