[Webkit-unassigned] [Bug 23499] Add WML specific logic for WML <input>

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Sat Jan 24 01:17:49 PST 2009


https://bugs.webkit.org/show_bug.cgi?id=23499


yichao.yin at torchmobile.com.cn changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
  Attachment #26967|0                           |1
        is obsolete|                            |
  Attachment #26994|                            |review?
               Flag|                            |




------- Comment #8 from yichao.yin at torchmobile.com.cn  2009-01-24 01:17 PDT -------
Created an attachment (id=26994)
 --> (https://bugs.webkit.org/attachment.cgi?id=26994&action=view)
updated patch

updated patch which fixes some issues Niko points out.


Addtional explain for Niko's comments

> > +    static const String s_formatCodes;
> I'm not sure wheter it's wise to declare a static const String.
> What you could do is add a private function: "static const AtomicString&
> formatCodes();"
> const AtomicString& WMLInputElement::formatCodes()
> {
>     DEFINE_STATIC_LOCAL(AtomicString, codes, ("AaNnXxMm"));
>     return codes;
> }

Since formatCodes() is private and independent, I think it's better add the
function "static const AtomicString& formatCodes() const" in the source file
instead of defining it as a member of HTMLInputElement.



> > @@ -215,6 +235,9 @@ void WMLInputElement::attach()
> > +    // Here we initialize the WML <input> element
> > +    init();
> I'm not sure if it's desired to call init() from attach().
> How about calling it from 'finishedParsingChildren()' ?
> The drawback using attach() would be that I could force the renderer to be
> attached, detach and re-attached.
> This way we'd store the variable state twice, that's not what we want I fear.

>From functionality, placing it in finishedParsingChildren() is ok under most
situations. but the run-webkit-tests script can't get the expected rendering
result similar with that QtLauncher gets. As a result some cases will fail. But
if I place it in attach(), both of them works well. It seems the layout test
frameworks for WML needs calling init() multiple times to make sure the
rendering tree is right. exactly, ensure the vaiable value is correct. Anyway I
still keep it in attach() to make everthing is ok for now.


-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug, or are watching the assignee.



More information about the webkit-unassigned mailing list