Closed Bug 690372 Opened 9 years ago Closed 9 years ago

Give an IID to dom::Element

Categories

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

defect
Not set

Tracking

()

RESOLVED FIXED
mozilla11

People

(Reporter: kaze, Assigned: kaze)

References

Details

Attachments

(1 file, 1 obsolete file)

We want to use nsCOMPtr<dom::Element> instead of nsCOMPtr<nsIDOMElement> in a few cases, e.g. for bug 684187.
Assignee: nobody → kaze
Attached patch patch proposal (obsolete) — Splinter Review
Blocks: 684187
Status: NEW → ASSIGNED
Comment on attachment 563431 [details] [diff] [review]
patch proposal

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

::: content/base/public/Element.h
@@ +204,5 @@
>  
>    nsEventStates mState;
>  };
>  
> +NS_DEFINE_STATIC_IID_ACCESSOR(Element, NS_ELEMENT_IID)

I _think_ this should go into a cpp file.  nsGenericElement.cpp sounds like a good choice.
No, that typically goes in the header (e.g. see nsINode.h).
Comment on attachment 563431 [details] [diff] [review]
patch proposal

Right!  Sorry :-)
Attachment #563431 - Flags: review+
Sorry, I backed this out on inbound: https://hg.mozilla.org/integration/mozilla-inbound/rev/d1b5c48c22c7

because of crashes in test_fullscreen-api.html: https://tbpl.mozilla.org/php/getParsedLog.php?id=6632003&tree=Mozilla-Inbound
Target Milestone: mozilla10 → ---
Depends on: 688547
Attached patch patch proposalSplinter Review
(adding NS_INTERFACE_MAP_ENTRY)

This passes test_fullscreen-api.html here (Ubuntu 11.10).
Attachment #563431 - Attachment is obsolete: true
Attachment #568041 - Flags: review?(ehsan)
Comment on attachment 568041 [details] [diff] [review]
patch proposal

Ah, good catch!

Have this gone through try?
Attachment #568041 - Flags: review?(ehsan) → review+
Component: General → DOM
QA Contact: general → general
https://tbpl.mozilla.org/?tree=Try&rev=5230666d62db

The tests on Windows are not significant on this page (“infrastructure exception”). Anyway, there’s a fresh tbpl for bug 684187 which includes this patch: https://tbpl.mozilla.org/?tree=Try&rev=907654f23f5e
Component: DOM → General
Component: General → DOM
Keywords: checkin-needed
Flags: in-testsuite?
Keywords: checkin-needed
Target Milestone: --- → mozilla11
Attachment #568041 - Flags: checkin+
Has this landed?  It seems like this patch is not in yet.  In that case, what are we waiting on?
(In reply to Ehsan Akhgari [:ehsan] from comment #11)
> Has this landed?  It seems like this patch is not in yet.  In that case,
> what are we waiting on?

It's in inbound.
https://hg.mozilla.org/mozilla-central/rev/35b60f8565a7
Status: ASSIGNED → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
Depends on: 361460
Component: DOM → DOM: Core & HTML
You need to log in before you can comment on or make changes to this bug.