Implement the NamedGetter functionality on HTMLDocument

RESOLVED FIXED in mozilla22

Status

()

defect
RESOLVED FIXED
7 years ago
5 months ago

People

(Reporter: bzbarsky, Assigned: peterv)

Tracking

(Blocks 1 bug, {dev-doc-needed})

unspecified
mozilla22
x86
macOS
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(5 attachments, 4 obsolete attachments)

895 bytes, text/html
Details
3.42 KB, patch
peterv
: review+
Details | Diff | Splinter Review
4.28 KB, patch
bzbarsky
: review+
Details | Diff | Splinter Review
5.48 KB, patch
bzbarsky
: review+
Details | Diff | Splinter Review
4.08 KB, patch
bzbarsky
: review+
Details | Diff | Splinter Review
No description provided.
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...
Attachment #691181 - Flags: review?(peterv)
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...
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.  :(
Summary: Implement the NameGetter functionality on HTMLDocument and turn on WebIDL bindings for it → Implement the NamedGetter functionality on HTMLDocument and turn on WebIDL bindings for it
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.
I already have that part working.
Assignee: nobody → peterv
Depends on: 841447
We also need to reimplement .all.
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?
Duplicate of this bug: 706131
Peter, what's our plan for nsIPluginDocument and nsIImageDocument?  I guess we could just introduce extra interfaces to hold the relevant methods...
Updated to trunk. r=me.
Attachment #691181 - Attachment is obsolete: true
Attachment #691181 - Flags: review?(peterv)
Attachment #729529 - Flags: review+
This mostly just moves and reindents code.
Attachment #726127 - Attachment is obsolete: true
Attachment #729533 - Flags: review?(bzbarsky)
Doing this in the NamedGetter for now. We can design a better solution in the future.
Attachment #729535 - Flags: review?(bzbarsky)
Summary: Implement the NamedGetter functionality on HTMLDocument and turn on WebIDL bindings for it → Implement the NamedGetter functionality on HTMLDocument
Comment on attachment 691184 [details] [diff] [review]
part 2.  Turn on WebIDL bindings for HTMLDocument.

Moving this to a separate bug.
Attachment #691184 - Attachment is obsolete: true
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 on attachment 729531 [details] [diff] [review]
Implement GetSupportedNames on HTMLDocument

r=me
Attachment #729531 - Flags: review?(bzbarsky) → review+
Comment on attachment 729533 [details] [diff] [review]
Refactor code to support document.all

r=me, I guess
Attachment #729533 - Flags: review?(bzbarsky) → review+
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.
Attachment #729535 - Flags: review?(bzbarsky) → review+
No longer depends on: 841447
Component: DOM → DOM: Core & HTML
You need to log in before you can comment on or make changes to this bug.