Bug 23015 - RenderTextControl relies on HTMLFormControlElement, making it hard to reuse for WML
Summary: RenderTextControl relies on HTMLFormControlElement, making it hard to reuse f...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Forms (show other bugs)
Version: 528+ (Nightly build)
Hardware: Mac OS X 10.5
: P2 Normal
Assignee: Nikolas Zimmermann
URL:
Keywords:
Depends on:
Blocks: 20393
  Show dependency treegraph
 
Reported: 2008-12-28 14:36 PST by Nikolas Zimmermann
Modified: 2008-12-28 16:17 PST (History)
0 users

See Also:


Attachments
Initial patch (11.83 KB, patch)
2008-12-28 14:59 PST, Nikolas Zimmermann
sam: review-
Details | Formatted Diff | Diff
Updated patch (17.28 KB, patch)
2008-12-28 16:09 PST, Nikolas Zimmermann
koivisto: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Nikolas Zimmermann 2008-12-28 14:36:51 PST
RenderTextControl relies on HTMLFormControlElement, and is not usable within WML at the moment.

One possibility would be to introduce a FormControlElement abstract class, containing two pure-virtual functions: setValueMatchesRenderer/valueMatchesRenderer, as these methods are in use from within RenderTextControl. As RenderObject stores a Node* pointer only, we'd also need to store a FormControlElement pointer in RenderTextControl, as we aren't allowed to cast node() to HTMLFormControlElement, as we want to abstract away the HTML-dependency.

As Node got enough unused bits left, it's easier to store "bool m_controlValueMatchesRenderer : 1" in Node and add non-virtual getter/setter methods. This way RenderTextControl saves all casts from Node -> HTMLFormControlElement and the HTML dependency is gone.
Comment 1 Nikolas Zimmermann 2008-12-28 14:59:49 PST
Created attachment 26283 [details]
Initial patch
Comment 2 Sam Weinig 2008-12-28 15:26:50 PST
Comment on attachment 26283 [details]
Initial patch

It's a little sad to be using bits in Node for something as rare as a RenderTextControl.  Otherwise, the patch seems fine. r=me
Comment 3 Nikolas Zimmermann 2008-12-28 15:50:45 PST
As discussed with Antti & Sam, adding memory to RenderTextControl is not as bad as adding too specialized data (only needed for form control elements) to Node. I'll upload a revised version, implementing my first idea: a shared ABC.
Comment 4 Nikolas Zimmermann 2008-12-28 16:09:35 PST
Created attachment 26285 [details]
Updated patch

Found a much nicer way to refactor, without adding any extra memory consumption.
Comment 5 Nikolas Zimmermann 2008-12-28 16:17:09 PST
Landed in r39495.