[PATCH v2 1/8] mshtml: Implement IHTMLElement::toString.
Gabriel Ivăncescu
gabrielopcode at gmail.com
Mon Sep 27 11:54:48 CDT 2021
Hi Jacek,
On 27/09/2021 18:42, Jacek Caban wrote:
> Hi Gabriel,
>
> On 9/24/21 3:45 PM, Gabriel Ivăncescu wrote:
>> Signed-off-by: Gabriel Ivăncescu <gabrielopcode at gmail.com>
>> ---
>> dlls/mshtml/dispex.c | 10 ++
>> dlls/mshtml/htmlanchor.c | 3 +-
>> dlls/mshtml/htmlarea.c | 3 +-
>> dlls/mshtml/htmlbody.c | 3 +-
>> dlls/mshtml/htmlcomment.c | 3 +-
>> dlls/mshtml/htmlelem.c | 163 +++++++++++++++++++++++++-----
>> dlls/mshtml/htmlform.c | 3 +-
>> dlls/mshtml/htmlframe.c | 6 +-
>> dlls/mshtml/htmlhead.c | 12 ++-
>> dlls/mshtml/htmlimg.c | 3 +-
>> dlls/mshtml/htmlinput.c | 9 +-
>> dlls/mshtml/htmllink.c | 3 +-
>> dlls/mshtml/htmlobject.c | 6 +-
>> dlls/mshtml/htmlscript.c | 3 +-
>> dlls/mshtml/htmlselect.c | 6 +-
>> dlls/mshtml/htmlstyleelem.c | 3 +-
>> dlls/mshtml/htmltable.c | 9 +-
>> dlls/mshtml/htmltextarea.c | 3 +-
>> dlls/mshtml/mshtml_private.h | 2 +
>> dlls/mshtml/tests/documentmode.js | 142 ++++++++++++++++++++++++++
>> 20 files changed, 341 insertions(+), 54 deletions(-)
>>
>> diff --git a/dlls/mshtml/dispex.c b/dlls/mshtml/dispex.c
>> index 64ead8f..797f024 100644
>> --- a/dlls/mshtml/dispex.c
>> +++ b/dlls/mshtml/dispex.c
>> @@ -1482,6 +1482,16 @@ compat_mode_t dispex_compat_mode(DispatchEx
>> *dispex)
>> : dispex->info->desc->vtbl->get_compat_mode(dispex);
>> }
>> +const WCHAR *dispex_tostring(DispatchEx *dispex)
>> +{
>> + const WCHAR *str = dispex->info->desc->tostring_name;
>> +
>> + /* Empty string is special for all modes */
>> + if((!str || str[0]) && dispex_compat_mode(dispex) < COMPAT_MODE_IE9)
>
>
> The special case for an empty string should not be needed.
>
>
How should I handle it for the elements that return empty string even in
old modes (that would otherwise return [object])? For <a> and <area>
elements, they return empty string in *all* modes.
Also I'd have to special case either this, or having it set to NULL
(since I'll use a separate constructor now), because as you noted below,
the decoration must also not be added to it.
i.e. it literally returns empty string, not [object] or anything else.
>> + str = L"[object]";
>> + return str;
>> +}Gabriel
>
>
> I was hoping that it could take care of entire implementation. Caller
> could be about to do just:
>
> return dispex_to_string(&This->dispex, p);
>
> >> HRESULT HTMLCommentElement_Create(HTMLDocumentNode *doc, nsIDOMNode
>> *nsnode, HTMLElement **elem)
>> diff --git a/dlls/mshtml/htmlelem.c b/dlls/mshtml/htmlelem.c
>> index d9554f4..8546c19 100644
>> --- a/dlls/mshtml/htmlelem.c
>> +++ b/dlls/mshtml/htmlelem.c
>> @@ -47,31 +47,116 @@ typedef struct {
>> } tag_desc_t;
>> static const tag_desc_t tag_descs[] = {
>> - {L"A", HTMLAnchorElement_Create},
>> - {L"AREA", HTMLAreaElement_Create},
>> - {L"BODY", HTMLBodyElement_Create},
>> - {L"BUTTON", HTMLButtonElement_Create},
>> - {L"EMBED", HTMLEmbedElement_Create},
>> - {L"FORM", HTMLFormElement_Create},
>> - {L"FRAME", HTMLFrameElement_Create},
>> - {L"HEAD", HTMLHeadElement_Create},
>> - {L"HTML", HTMLHtmlElement_Create},
>> - {L"IFRAME", HTMLIFrame_Create},
>> - {L"IMG", HTMLImgElement_Create},
>> - {L"INPUT", HTMLInputElement_Create},
>> - {L"LABEL", HTMLLabelElement_Create},
>> - {L"LINK", HTMLLinkElement_Create},
>> - {L"META", HTMLMetaElement_Create},
>> - {L"OBJECT", HTMLObjectElement_Create},
>> - {L"OPTION", HTMLOptionElement_Create},
>> - {L"SCRIPT", HTMLScriptElement_Create},
>> - {L"SELECT", HTMLSelectElement_Create},
>> - {L"STYLE", HTMLStyleElement_Create},
>> - {L"TABLE", HTMLTable_Create},
>> - {L"TD", HTMLTableCell_Create},
>> - {L"TEXTAREA", HTMLTextAreaElement_Create},
>> - {L"TITLE", HTMLTitleElement_Create},
>> - {L"TR", HTMLTableRow_Create}
>> + {L"A", HTMLAnchorElement_Create},
>> + {L"ABBR", NULL},
>> + {L"ACRONYM", NULL},
>> + {L"ADDRESS", NULL},
>> + {L"APPLET", NULL},
>> + {L"AREA", HTMLAreaElement_Create},
>> + {L"ARTICLE", NULL},
>> + {L"ASIDE", NULL},
>> + {L"AUDIO", NULL},
>> + {L"B", NULL},
>> + {L"BASE", NULL},
>> + {L"BASEFONT", NULL},
>> + {L"BDO", NULL},
>> + {L"BIG", NULL},
>> + {L"BLOCKQUOTE", NULL},
>> + {L"BODY", HTMLBodyElement_Create},
>> + {L"BR", NULL},
>> + {L"BUTTON", HTMLButtonElement_Create},
>> + {L"CANVAS", NULL},
>> + {L"CAPTION", NULL},
>> + {L"CENTER", NULL},
>> + {L"CITE", NULL},
>> + {L"CODE", NULL},
>> + {L"COL", NULL},
>> + {L"COLGROUP", NULL},
>> + {L"DATALIST", NULL},
>> + {L"DD", NULL},
>> + {L"DEL", NULL},
>> + {L"DFN", NULL},
>> + {L"DIR", NULL},
>> + {L"DIV", NULL},
>> + {L"DL", NULL},
>> + {L"DT", NULL},
>> + {L"EM", NULL},
>> + {L"EMBED", HTMLEmbedElement_Create},
>> + {L"FIELDSET", NULL},
>> + {L"FIGCAPTION", NULL},
>> + {L"FIGURE", NULL},
>> + {L"FONT", NULL},
>> + {L"FOOTER", NULL},
>> + {L"FORM", HTMLFormElement_Create},
>> + {L"FRAME", HTMLFrameElement_Create},
>> + {L"FRAMESET", NULL},
>> + {L"H1", NULL},
>> + {L"H2", NULL},
>> + {L"H3", NULL},
>> + {L"H4", NULL},
>> + {L"H5", NULL},
>> + {L"H6", NULL},
>> + {L"HEAD", HTMLHeadElement_Create},
>> + {L"HEADER", NULL},
>> + {L"HR", NULL},
>> + {L"HTML", HTMLHtmlElement_Create},
>> + {L"I", NULL},
>> + {L"IFRAME", HTMLIFrame_Create},
>> + {L"IMG", HTMLImgElement_Create},
>> + {L"INPUT", HTMLInputElement_Create},
>> + {L"INS", NULL},
>> + {L"KBD", NULL},
>> + {L"LABEL", HTMLLabelElement_Create},
>> + {L"LEGEND", NULL},
>> + {L"LI", NULL},
>> + {L"LINK", HTMLLinkElement_Create},
>> + {L"MAP", NULL},
>> + {L"MARK", NULL},
>> + {L"META", HTMLMetaElement_Create},
>> + {L"NAV", NULL},
>> + {L"NOFRAMES", NULL},
>> + {L"NOSCRIPT", NULL},
>> + {L"OBJECT", HTMLObjectElement_Create},
>> + {L"OL", NULL},
>> + {L"OPTGROUP", NULL},
>> + {L"OPTION", HTMLOptionElement_Create},
>> + {L"P", NULL},
>> + {L"PARAM", NULL},
>> + {L"PRE", NULL},
>> + {L"PROGRESS", NULL},
>> + {L"Q", NULL},
>> + {L"RP", NULL},
>> + {L"RT", NULL},
>> + {L"RUBY", NULL},
>> + {L"S", NULL},
>> + {L"SAMP", NULL},
>> + {L"SCRIPT", HTMLScriptElement_Create},
>> + {L"SECTION", NULL},
>> + {L"SELECT", HTMLSelectElement_Create},
>> + {L"SMALL", NULL},
>> + {L"SOURCE", NULL},
>> + {L"SPAN", NULL},
>> + {L"STRIKE", NULL},
>> + {L"STRONG", NULL},
>> + {L"STYLE", HTMLStyleElement_Create},
>> + {L"SUB", NULL},
>> + {L"SUP", NULL},
>> + {L"TABLE", HTMLTable_Create},
>> + {L"TBODY", NULL},
>> + {L"TD", HTMLTableCell_Create},
>> + {L"TEXTAREA", HTMLTextAreaElement_Create},
>> + {L"TFOOT", NULL},
>> + {L"TH", NULL},
>> + {L"THEAD", NULL},
>> + {L"TITLE", HTMLTitleElement_Create},
>> + {L"TR", HTMLTableRow_Create},
>> + {L"TRACK", NULL},
>> + {L"TT", NULL},
>> + {L"U", NULL},
>> + {L"UL", NULL},
>> + {L"VAR", NULL},
>> + {L"VIDEO", NULL},
>> + {L"WBR", NULL}
>> };
>
>
> I think that having a separated constructor instead of all those NULLs
> would avoid the mess that you need in HTMLElement_toString:
>
>
Okay, I'm just not sure how to deal with the use_generic thing, since
right now it uses a different constructor/vtbl. Do I have to duplicate
that, too? I don't even know what it's used for.
Or just ignore it? What should I base the new constructor on, the
generic one (HTMLGenericElement_dispex), or the HTMLElement_dispex?
>> static const tag_desc_t *get_tag_desc(const WCHAR *tag_name)
>> @@ -2112,8 +2197,30 @@ static HRESULT WINAPI
>> HTMLElement_get_ondragstart(IHTMLElement *iface, VARIANT *
>> static HRESULT WINAPI HTMLElement_toString(IHTMLElement *iface, BSTR
>> *String)
>> {
>> HTMLElement *This = impl_from_IHTMLElement(iface);
>> - FIXME("(%p)->(%p)\n", This, String);
>> - return E_NOTIMPL;
>> + const PRUnichar *str;
>> + nsAString tag_str;
>> + nsresult nsres;
>> +
>> + TRACE("(%p)->(%p)\n", This, String);
>> +
>> + if(!String)
>> + return E_INVALIDARG;
>> +
>> + str = dispex_tostring(&This->node.event_target.dispex);
>> + if(!str) {
>> + nsAString_Init(&tag_str, NULL);
>> + nsres = nsIDOMElement_GetTagName(This->dom_element, &tag_str);
>> + if(NS_FAILED(nsres)) {
>> + nsAString_Finish(&tag_str);
>> + return map_nsresult(nsres);
>> + }
>> + nsAString_GetData(&tag_str, &str);
>> + str = get_tag_desc(str) ? L"[object HTMLElement]" : L"[object
>> HTMLUnknownElement]";
>> + nsAString_Finish(&tag_str);
>> + }
>> +
>> + *String = SysAllocString(str);
>> + return *String ? S_OK : E_OUTOFMEMORY;
>> }
>
>
>
> (...)
>
>
>> diff --git a/dlls/mshtml/mshtml_private.h b/dlls/mshtml/mshtml_private.h
>> index c1e7e78..67ea0ad 100644
>> --- a/dlls/mshtml/mshtml_private.h
>> +++ b/dlls/mshtml/mshtml_private.h
>> @@ -331,6 +331,7 @@ typedef struct {
>> const tid_t disp_tid;
>> const tid_t* const iface_tids;
>> void (*init_info)(dispex_data_t*,compat_mode_t);
>> + const WCHAR *tostring_name;
>> dispex_data_t *info_cache[COMPAT_MODE_CNT];
>> dispex_data_t *delayed_init_info;
>> } dispex_static_data_t;
>
>
> It would be cleaner to no tie it so tightly with toString and store just
> the type name. "[object ...]" decorations can be a part of toString
> implementation.
>
>
> I also think it would be nicer to have it as the first field of
> dispex_static_data_t.
>
>
> Thanks,
>
> Jacek
>
Noted.
Thanks,
Gabriel
More information about the wine-devel
mailing list