[Webkit-unassigned] [Bug 20393] Add WML support to WebKit

[bugzilla-daemon at webkit.org]

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





------- Comment #28 from yichao.yin at torchmobile.com.cn  2008-11-14 01:56 PDT -------

Thank zimmermann for your comments.


(In reply to comment #26)
> HTMLSelectElement:
> >  void HTMLSelectElement::setMultiple(bool multiple)
> >  {
> > +#if ENABLE(WML)
> > +    m_multiple = multiple;
> > +#else
> >      setAttribute(multipleAttr, multiple ? "" : 0);
> > +#endif
> >  }
> When enabling WML, this would affect HTML as well, is altering m_multiple
> really desired? When looking through the code, m_multiple is set through
> parseMappedAttribute, and in general most HTMLElement derived classes exposing
> setXXXValue / xxxValue functions are supposed to modify the corresponding
> attribute values through setAttribute. Alterin m_multiple violates this
> pattern. What's the reason?

* Yes, we'd better set value of class member with setXXXX() method by
convetion.
In some cases WMLSelectElement needs to update the
HTMLSelectElement::m_multiple but HTMLSelectElement::setMultiple() doesn't
support it. I think we needn't to add new method for HTMLSelectElement and
WMLSelectElement to implement that. 
Of cource, you are right. we should  not affect HTML while WML is enabled. I
have fixed it in my new patch.


> MainResourceLoader.cpp:
> > +#if ENABLE(WBXML)
> > +    if (length && m_response.mimeType() == "application/vnd.wap.wmlc") {
> > +        WB_UTINY *wbxml = (WB_UTINY*)data;
> > +        WB_ULONG wbxml_len = length;
> > +        WB_ULONG xml_len = 0;
> > +        WB_UTINY *xml = 0;
> > +        WBXMLError ret = WBXML_OK; 
> > +        WBXMLGenXMLParams params;
> > + 
> > +        // Init Default Parameters
> > +        params.lang = WBXML_LANG_UNKNOWN;
> > +        params.gen_type = WBXML_GEN_XML_INDENT;
> > +        params.indent = 1;
> > +        params.keep_ignorable_ws = FALSE;
> > + 
> > +        ret = wbxml_conv_wbxml2xml_withlen(wbxml, wbxml_len, &xml, &xml_len, &params);
> > +        if (ret != WBXML_OK)
> > +            return;
> > +        data = (char*)xml;
> > +        length = xml_len;
> > +    }
> > +#endif
> That looks hackish overall. Please move stars -> "WB_UTINY *wbxml" ->
> "WB_UTINY* wbxml".
> C style casts should be changed to static/reinterpret cast.

* Thanks for your advice and new patch. I did slight change for your update
about this issue because (const char*)data can't be casted to WB_UTINY*
directly.


-- 
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.