Re: [PATCH 6/7] mshtml: Allocate DISPIDs only for actual attributes in HTMLAttributeCollection.

[Gabriel Ivăncescu]

On 04/10/2021 19:32, Jacek Caban wrote:
> On 10/1/21 3:12 PM, Gabriel Ivăncescu wrote:
> > Signed-off-by: Gabriel Ivăncescu
> >
> > Presently, even toString() is exposed as an attribute, because it's a
> > builtin of the element, which makes no sense.
>
> Unfortunately, it's more tricky than that. With that change, we'd expose 
> too few attributes (instead of too many), which is likely to cause 
> regressions. In general, attributes objects were not yet updated for 
> compatibility modes and they are likely to require more drastic changes. 
> I would expect attributes to be proper nodes in IE9+ mode (rather than 
> node alike objects from previous versions), so we could probably make 
> them a wrappers around nsIDOMAttr using HTMLDOMNode. Similarly, it would 
> be interesting to try to use nsIDOMMozNamedAttrMap for 
> HTMLAttributeCollection.
>
> I'm not sure why it matters for toString, it seems like none of that 
> should be required to get toString itself working.
>
> Thanks,
>
> Jacek

Well it's needed to test the object name and toString for attributes 
object, but if it's that complicated I'll let it aside for the moment 
and just drop it from the tests somehow and add a note (it's only one 
line anyway).

Basically what happens is this:

Test function (later, not sent yet) tries toString and checks it. Works 
(because of the toString "attribute" wrongly exposed from the element) 
so it retrieves it as a DISPID. So it ends up in 
HTMLAttributeCollection_invoke with DISPATCH_METHOD (i.e. calling it on 
an "attribute" named "toString" as a method since it's .toString() in 
the test code) which is of course not implemented and causes an 
exception. It's not the unimplemented issue though, I mean, does it even 
make sense to call an attribute with DISPATCH_METHOD?

Anyway like I said I'll just drop the test with a note and you can 
ignore this patch for now.

Thanks,
Gabriel