Bug 95939 - [Refactoring] ButtonInputType of <input> element should have innerElement to make <input> AuthorShadowDOM-ready
Summary: [Refactoring] ButtonInputType of <input> element should have innerElement to ...
Status: RESOLVED INVALID
Alias: None
Product: WebKit
Classification: Unclassified
Component: DOM (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Web Components Team
URL:
Keywords:
Depends on: 97312
Blocks: 92608
  Show dependency treegraph
 
Reported: 2012-09-06 00:07 PDT by Shinya Kawanaka
Modified: 2013-06-18 21:22 PDT (History)
8 users (show)

See Also:


Attachments
WIP (211.38 KB, patch)
2012-09-06 04:34 PDT, Shinya Kawanaka
no flags Details | Formatted Diff | Diff
Patch (11.54 KB, patch)
2012-09-06 23:41 PDT, Shinya Kawanaka
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
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.