Bug 95939

Summary: [Refactoring] ButtonInputType of <input> element should have innerElement to make <input> AuthorShadowDOM-ready
Product: WebKit Reporter: Shinya Kawanaka <shinyak>
Component: DOMAssignee: Web Components Team <webcomponents-bugzilla>
Status: RESOLVED INVALID    
Severity: Normal CC: adele, arv, eric, mifenton, morrita, tkent, webcomponents-bugzilla, webkit.review.bot
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Bug Depends on: 97312    
Bug Blocks: 92608    
Attachments:
Description Flags
WIP
none
Patch none

Description Shinya Kawanaka 2012-09-06 00:07:31 PDT
ButtonInputType creates a ButtonRenderer, and its text is rendered using ButtonRenderer::setText.
Since the renderer of its text is always appended as last child of RenderButton, we cannot get text for <shadow> element.

We should create some inner element for ButtonInputType to make <input> AuthorShadowDOM ready.
Comment 1 Shinya Kawanaka 2012-09-06 04:34:45 PDT
Created attachment 162474 [details]
WIP
Comment 2 Shinya Kawanaka 2012-09-06 05:33:31 PDT
Needs a lot of rebaseline

and a few tests have image diff. We have to fix them.
editing/selection/3690703.html
Comment 3 Dimitri Glazkov (Google) 2012-09-06 09:34:25 PDT
Comment on attachment 162474 [details]
WIP

Wow, this patch certainly cleans up the guts of the RenderButton nicely! Do we still need the RenderButton?
Comment 4 Erik Arvidsson 2012-09-06 10:53:46 PDT
Comment on attachment 162474 [details]
WIP

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

> Source/WebCore/html/BaseButtonInputType.cpp:70
> +    element()->userAgentShadowRoot()->replaceChild(text, element()->userAgentShadowRoot()->firstChild());

Is there a reason why this uses replaceChild instead of updating the existing text node using setData
Comment 5 Shinya Kawanaka 2012-09-06 19:18:01 PDT
(In reply to comment #4)
> (From update of attachment 162474 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=162474&action=review
> 
> > Source/WebCore/html/BaseButtonInputType.cpp:70
> > +    element()->userAgentShadowRoot()->replaceChild(text, element()->userAgentShadowRoot()->firstChild());
> 
> Is there a reason why this uses replaceChild instead of updating the existing text node using setData

Thanks! I've searched only Text class. I should have checked CharacterData.
Comment 6 Shinya Kawanaka 2012-09-06 19:23:14 PDT
(In reply to comment #3)
> (From update of attachment 162474 [details])
> Wow, this patch certainly cleans up the guts of the RenderButton nicely! Do we still need the RenderButton?

When we could remove m_inner block, we might be able to do it. It's beyond the scope of this patch, though.
Comment 7 Shinya Kawanaka 2012-09-06 22:31:19 PDT
Hmm. According to this discussion, we should not be able to select text in a button.
https://bugs.webkit.org/show_bug.cgi?id=13624
Comment 8 Shinya Kawanaka 2012-09-06 23:41:06 PDT
Created attachment 162682 [details]
Patch
Comment 9 Shinya Kawanaka 2012-09-06 23:42:05 PDT
Now we're using TextFragment.
Comment 10 WebKit Review Bot 2012-09-19 19:07:05 PDT
Comment on attachment 162682 [details]
Patch

Clearing flags on attachment: 162682

Committed r129086: <http://trac.webkit.org/changeset/129086>
Comment 11 WebKit Review Bot 2012-09-19 19:07:10 PDT
All reviewed patches have been landed.  Closing bug.
Comment 12 WebKit Review Bot 2012-09-21 02:18:05 PDT
Re-opened since this is blocked by 97312
Comment 13 Shinya Kawanaka 2012-09-21 02:19:44 PDT
Check this https://bugs.webkit.org/show_bug.cgi?id=97310
Comment 14 Kent Tamura 2013-06-18 21:22:22 PDT
Shadow DOM API was removed.