Give an IID to dom::Element

RESOLVED FIXED in mozilla11

Status

()

Core
DOM
RESOLVED FIXED
6 years ago
5 years ago

People

(Reporter: kaze, Assigned: kaze)

Tracking

Trunk
mozilla11
Points:
---
Dependency tree / graph
Bug Flags:
in-testsuite ?

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 1 obsolete attachment)

(Assignee)

Description

6 years ago
We want to use nsCOMPtr<dom::Element> instead of nsCOMPtr<nsIDOMElement> in a few cases, e.g. for bug 684187.
(Assignee)

Updated

6 years ago
Assignee: nobody → kaze
(Assignee)

Comment 1

6 years ago
Created attachment 563431 [details] [diff] [review]
patch proposal
(Assignee)

Updated

6 years ago
Blocks: 684187
(Assignee)

Updated

6 years ago
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+
https://hg.mozilla.org/integration/mozilla-inbound/rev/5336c87067f7
Target Milestone: --- → mozilla10
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 → ---
http://mxr.mozilla.org/mozilla-central/source/content/base/src/nsDocument.cpp#8633

(Which I just happen to have a fix for in bug 688547)
Depends on: 688547
(Assignee)

Comment 8

6 years ago
Created attachment 568041 [details] [diff] [review]
patch proposal

(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
(Assignee)

Comment 10

6 years ago
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
(Assignee)

Updated

6 years ago
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
Last Resolved: 6 years ago
Resolution: --- → FIXED

Updated

5 years ago
Depends on: 361460
You need to log in before you can comment on or make changes to this bug.