Last Comment Bug 690372 - Give an IID to dom::Element
: Give an IID to dom::Element
Product: Core
Classification: Components
Component: DOM (show other bugs)
: Trunk
: All All
: -- normal (vote)
: mozilla11
Assigned To: Fabien Cazenave [:kaze]
: Andrew Overholt [:overholt]
Depends on: 361460 688547
Blocks: 684187
  Show dependency treegraph
Reported: 2011-09-29 08:44 PDT by Fabien Cazenave [:kaze]
Modified: 2012-04-30 01:11 PDT (History)
7 users (show)
mounir: in‑testsuite?
See Also:
Crash Signature:
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---

patch proposal (1.66 KB, patch)
2011-09-29 09:00 PDT, Fabien Cazenave [:kaze]
ehsan: review+
Details | Diff | Splinter Review
patch proposal (2.57 KB, patch)
2011-10-19 07:22 PDT, Fabien Cazenave [:kaze]
ehsan: review+
mounir: checkin+
Details | Diff | Splinter Review

Description Fabien Cazenave [:kaze] 2011-09-29 08:44:57 PDT
We want to use nsCOMPtr<dom::Element> instead of nsCOMPtr<nsIDOMElement> in a few cases, e.g. for bug 684187.
Comment 1 Fabien Cazenave [:kaze] 2011-09-29 09:00:39 PDT
Created attachment 563431 [details] [diff] [review]
patch proposal
Comment 2 :Ehsan Akhgari 2011-09-29 10:18:33 PDT
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;
>  };

I _think_ this should go into a cpp file.  nsGenericElement.cpp sounds like a good choice.
Comment 3 Boris Zbarsky [:bz] (still a bit busy) 2011-09-29 20:46:31 PDT
No, that typically goes in the header (e.g. see nsINode.h).
Comment 4 :Ehsan Akhgari 2011-09-30 15:41:23 PDT
Comment on attachment 563431 [details] [diff] [review]
patch proposal

Right!  Sorry :-)
Comment 6 Matt Brubeck (:mbrubeck) 2011-09-30 17:36:20 PDT
Sorry, I backed this out on inbound:

because of crashes in test_fullscreen-api.html:
Comment 7 :Ms2ger (⌚ UTC+1/+2) 2011-10-01 02:43:17 PDT

(Which I just happen to have a fix for in bug 688547)
Comment 8 Fabien Cazenave [:kaze] 2011-10-19 07:22:13 PDT
Created attachment 568041 [details] [diff] [review]
patch proposal


This passes test_fullscreen-api.html here (Ubuntu 11.10).
Comment 9 :Ehsan Akhgari 2011-10-19 07:35:22 PDT
Comment on attachment 568041 [details] [diff] [review]
patch proposal

Ah, good catch!

Have this gone through try?
Comment 10 Fabien Cazenave [:kaze] 2011-10-19 08:00:05 PDT

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:
Comment 11 :Ehsan Akhgari 2011-11-21 11:51:11 PST
Has this landed?  It seems like this patch is not in yet.  In that case, what are we waiting on?
Comment 12 Mounir Lamouri (:mounir) 2011-11-21 11:58:25 PST
(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.
Comment 13 Ed Morley [:emorley] 2011-11-21 19:11:23 PST

Note You need to log in before you can comment on or make changes to this bug.