820657 - Implement the NamedGetter functionality on HTMLDocument

Status: RESOLVED FIXED

(Core :: DOM: Core & HTML, defect)

Comment 1

[Boris Zbarsky [:bzbarsky]]

Attachment: Implement the NamedGetter on HTMLDocument. some point here we should change our hashtable implementation to

only store the things we care about in ResolveName so we don't have to
do all the complicated filtering by tag name and whatnot: just do a
hashtable lookup and move on...

Comment 2

[Boris Zbarsky [:bzbarsky]]

Attachment: part 2. Turn on WebIDL bindings for HTMLDocument.

Comment 3

[Boris Zbarsky [:bzbarsky]]

Attachment: Performance/correctness testcase

Comment 4

[Boris Zbarsky [:bzbarsky]]

There's another testcase at https://bug-104623-attachments.webkit.org/attachment.cgi?id=178683 that can be used to test correctness and can get timing added to measure perf...

Comment 5

[Boris Zbarsky [:bzbarsky]]

Attachment: Er, real perf/correctness testcase

Comment 6

[Boris Zbarsky [:bzbarsky]]

Note that we'll need to make the proxy handler for this stuff deal with the [Unforgeable] location.  Right now this patch fails codegen even with bug 821438 fixed.  :(

Comment 7

[Boris Zbarsky [:bzbarsky]]

We'll also need the proxy handler to expose names when called via an Xray but pretend to not be [OverrideBuiltins] in that case...  Might not be too bad because Xrays skip the expando get anyway, so it'll just be a matter of checking up the proto chain like we do for most handlers, but only in the Xray case.

Comment 8

[Peter Van der Beken [:peterv]]

I already have that part working.

Comment 9

[Peter Van der Beken [:peterv]]

We also need to reimplement .all.

Comment 10

[Boris Zbarsky [:bzbarsky]]

Hmm.  Just because the way it's hooked up right now is messed up?  That seems like a side-project that we need to do anyway; we need a separate bug, yes?

Comment 12

[Boris Zbarsky [:bzbarsky]]

Peter, what's our plan for nsIPluginDocument and nsIImageDocument?  I guess we could just introduce extra interfaces to hold the relevant methods...

Comment 13

[Peter Van der Beken [:peterv]]

Attachment: Support document.all

Comment 14

[Peter Van der Beken [:peterv]]

Attachment: Implement the NamedGetter on HTMLDocument

Updated to trunk. r=me.

Comment 15

[Peter Van der Beken [:peterv]]

Attachment: Implement GetSupportedNames on HTMLDocument

Comment 16

[Peter Van der Beken [:peterv]]

Attachment: Refactor code to support document.all

This mostly just moves and reindents code.

Comment 17

[Peter Van der Beken [:peterv]]

Attachment: Hook up document.all in the HTMLDocument NamedGetter

Doing this in the NamedGetter for now. We can design a better solution in the future.

Comment 18

[Peter Van der Beken [:peterv]]

Comment on attachment 691184 [details] [diff] [review]
part 2.  Turn on WebIDL bindings for HTMLDocument.

Moving this to a separate bug.

Comment 19

[:Ms2ger (he/him; ⌚ UTC+1/+2)]

Comment on attachment 729531 [details] [diff] [review]
Implement GetSupportedNames on HTMLDocument

Review of attachment 729531 [details] [diff] [review]:
-----------------------------------------------------------------

::: content/html/content/src/nsGenericHTMLElement.h
@@ +717,5 @@
>  
>    static bool TouchEventsEnabled(JSContext* /* unused */, JSObject* /* unused */);
>  
> +  static inline bool
> +  ExposeIdAsHTMLDocumentProperty(Element* aElement)

This sounds as if it would actively go and expose something. Maybe ShouldExpose...?

::: content/html/document/src/nsHTMLDocument.cpp
@@ +2395,4 @@
>  void
>  nsHTMLDocument::GetSupportedNames(nsTArray<nsString>& aNames)
>  {
>    // Nothing, for now.  We should fix that.

We fixed it!

Comment 20

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 729531 [details] [diff] [review]
Implement GetSupportedNames on HTMLDocument

r=me

Comment 21

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 729533 [details] [diff] [review]
Refactor code to support document.all

r=me, I guess

Comment 22

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 729535 [details] [diff] [review]
Hook up document.all in the HTMLDocument NamedGetter

This is icky, but ok for now.

Something here needs to enter the compartment of the wrapper on cx, right?  Either the NamedGetter or nsHTMLDocumentSH::TryResolveAll.

r=me with that.

Comment 23

[Peter Van der Beken [:peterv]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/8960293f0fb3
https://hg.mozilla.org/integration/mozilla-inbound/rev/a2e02690e9bd
https://hg.mozilla.org/integration/mozilla-inbound/rev/1d97c190fe24
https://hg.mozilla.org/integration/mozilla-inbound/rev/85bcef2ac1fd

I removed the getter from the WebIDL file for now, we need unforgeable support for proxies first.

Comment 24

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/8960293f0fb3
https://hg.mozilla.org/mozilla-central/rev/a2e02690e9bd
https://hg.mozilla.org/mozilla-central/rev/1d97c190fe24
https://hg.mozilla.org/mozilla-central/rev/85bcef2ac1fd