Update callers that use QueryInterface on XBL implemented or custom element interfaces

RESOLVED FIXED in Firefox 66

Status

()

task
P4
normal
RESOLVED FIXED
9 months ago
19 days ago

People

(Reporter: enndeakin, Assigned: enndeakin)

Tracking

(Blocks 1 bug)

unspecified
mozilla66
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox66 fixed)

Details

Attachments

(7 attachments, 9 obsolete attachments)

20.49 KB, patch
peterv
: review+
Details | Diff | Splinter Review
5.76 KB, patch
peterv
: review+
Details | Diff | Splinter Review
41.07 KB, patch
surkov
: review+
Details | Diff | Splinter Review
3.58 KB, patch
peterv
: review+
Details | Diff | Splinter Review
4.15 KB, patch
Paolo
: review+
Details | Diff | Splinter Review
10.25 KB, patch
Paolo
: review+
Details | Diff | Splinter Review
14.35 KB, patch
Paolo
: review+
Details | Diff | Splinter Review
Assignee

Description

9 months ago
They can be changed to use specific methods on Element or nsXULElement.

From bug 1478372:

"Change callers that currently QueryInterface to instead use CallGetCustomInterface via a custom method defined in Element (or nsXULElement)

Some specific callers could check tag names instead where the QI is only used to verify an interface rather than call a method on it.

The disadvantage is that some cases won't be handled. For example, some callers call nsIDOMXULSelectControlElement::GetSelectedItem and then QI the result to nsIContent, which doesn't work. There may be others that might be hard to track down without more extensive testing."
Assignee

Updated

9 months ago
Priority: -- → P4
Assignee

Comment 1

9 months ago
For this, I am working on a patch that changes the QueryInterface calls to use a method directly in Element/nsXULElement.

I also experimenting with a change to the nsIDOMXUL* interfaces so that the declarations only return Elements instead of other nsIDOMXUL* interfaces. This way I think they can be cast to nsIContent as accessibility code needs to do this.

So far there are a lot of failures to fix up:

https://treeherder.mozilla.org/#/jobs?repo=try&selectedJob=204800455&revision=fb31eb24f53acdc99d1990526bbf4d1d91516895
Could you confirm if this will ultimately work for nsIBrowser as well, so as to fix the assertion in https://bugzilla.mozilla.org/show_bug.cgi?id=1496067#c0? I see the try push doesn't include that interface.
Flags: needinfo?(enndeakin)
Assignee

Comment 3

8 months ago
Assignee: nobody → enndeakin
Status: NEW → ASSIGNED
Flags: needinfo?(enndeakin)
Assignee

Comment 8

8 months ago
These patches handle the nsIDOMXUL*, nsIDateTimeInputArea and nsIBrowser interfaces.

Still to go:

1. Remove nsIObserver and nsITimerCallback usages. These can be moved to just using a helper object I think.
2. Remove autocomplete interfaces. nsIAutoCompletePopup and nsIAutoCompleteInput. The latter is a bit trickier as nsIAutoCompleteInput is also implemented by the C++ object in nsFormFillController.cpp. Probably will just special-case this one.
Blocks: 1441935
I don't think we need to worry about nsIDateTimeInputArea - Tim, can you confirm that is going to be removed in Bug 1496242?
Flags: needinfo?(timdream)
Although if all callers need to be updated, and this is going to land before Bug 1496242 then I don't think there's a particular harm in switching nsIDateTimeInputArea.
datetimebox.js can never "bind" to the DOM as a custom element*. I don't think it would be able to implement nsIDateTimeInputArea without that ability.

(* we want to avoid declaring a custom element in web content HTML document)

Given that we can't keep nsIDateTimeInputArea, conversion is not going to be useful.
Flags: needinfo?(timdream)
Assignee

Comment 12

8 months ago
Some of these helpers would only be used in accessibility code. Might be better to move them there possibly.
Attachment #9018133 - Attachment is obsolete: true
Assignee

Comment 13

8 months ago
Attachment #9018134 - Attachment is obsolete: true
Attachment #9018135 - Attachment is obsolete: true
Attachment #9020111 - Flags: review?(surkov.alexander)
Assignee

Comment 16

8 months ago
Doing 'instanceof nsIDOMX*' doesn't work anymore. Neither does getting an inherited interface. Not sure if that is intentional, but this patch fixes issues relating to that.
Assignee

Updated

8 months ago
Attachment #9020117 - Flags: review?(paolo.mozmail)
Attachment #9020117 - Flags: feedback?(bgrinstead)
Comment on attachment 9020111 [details] [diff] [review]
Part 3 - Use Element helper methods in accessibility instead of QueryInterface in accessibility

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

::: accessible/generic/Accessible.cpp
@@ +1818,5 @@
>              // Check for button in list of default="true" elements
>              for (uint32_t count = 0; count < length && !buttonEl; count ++) {
> +              nsIContent* item = possibleDefaultButtons->Item(count);
> +              if (item->IsXULElement(nsGkAtoms::button) ||
> +                  item->IsXULElement(nsGkAtoms::toolbarbutton)) {

can toolbarbutton have @default attribute?

::: accessible/xul/XULComboboxAccessible.cpp
@@ +58,5 @@
>  
>    // Get focus status from base class
>    uint64_t state = Accessible::NativeState();
>  
> +  

nit: whitespace?

::: accessible/xul/XULMenuAccessible.cpp
@@ +76,5 @@
>    if (isComboboxOption) {
>      // Is selected?
>      bool isSelected = false;
> +    nsCOMPtr<nsIDOMXULSelectControlItemElement> item =
> +      mContent->AsElement()->AsXULSelectControlItem();

Elm()->AsXUL...

@@ +247,1 @@
>      return roles::PARENT_MENUITEM;

it should be safe, because we indeed want a menu here, but technically nsIDOMXULContainerElement is also implemented by tags other than xul:menu, for example, by xul:menucaption.

::: accessible/xul/XULSelectControlAccessible.cpp
@@ +107,5 @@
>    }
>  
>    // For XUL single-select control/menulist
> +  nsCOMPtr<nsIDOMXULSelectControlElement> selectControl =
> +    mSelectControl->AsXULSelectControl();

oh, multiselect in not inherited from selectcontrol anymore?

@@ +109,5 @@
>    // For XUL single-select control/menulist
> +  nsCOMPtr<nsIDOMXULSelectControlElement> selectControl =
> +    mSelectControl->AsXULSelectControl();
> +  if (selectControl) {
> +    int32_t index;

could pls initialize it?

@@ +126,4 @@
>      return false;
>  
>    nsCOMPtr<nsIDOMXULSelectControlItemElement> itemElm =
> +    item->GetContent()->AsElement()->AsXULSelectControlItem();

Accessible has Elm() method you can use here

@@ +144,5 @@
> +    nsCOMPtr<nsIDOMXULSelectControlElement> selectControl =
> +      mSelectControl->AsXULSelectControl();
> +    if (selectControl) {
> +      nsCOMPtr<Element> element = do_QueryInterface(item->GetContent());
> +      selectControl->SetSelectedItem(element);

can you do ->SetSelectedItem(item->Elm())?

::: accessible/xul/XULSelectControlAccessible.h
@@ +39,5 @@
>    virtual Accessible* CurrentItem() const override;
>    virtual void SetCurrentItem(const Accessible* aItem) override;
>  
>  protected:
> +  RefPtr<Element> mSelectControl;

don't you need dom:: prefix?
Attachment #9020111 - Flags: review?(surkov.alexander) → review+
Comment on attachment 9020117 [details] [diff] [review]
Part 6 - Use tagnames instead of nsIDOMXUL* interfaces in mobile/toolkit

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

::: toolkit/components/prompts/content/tabprompts.xml
@@ +334,5 @@
>                // when no other button has focus. XUL buttons on not-OSX will
>                // react to pressing enter as a command, so you can't trigger the
>                // default without tabbing to it or something that isn't a button.
>                let focusedDefault = (event.originalTarget == defaultButton);
> +              let someButtonFocused = event.originalTarget.localName == "button" ||

Would it be reasonable to add a property (either on the XBL/CE or on the XUL element through IDL) like `.isButtonElement` or similar?  Or make a getter on the element that _does_ implement the interface and then instanceof check on that? Then we wouldn't have to worry about keeping this list up to date as elements get added/removed from this set.
Attachment #9020117 - Flags: feedback?(bgrinstead)
Assignee

Updated

8 months ago
Attachment #9020115 - Attachment is patch: true
Attachment #9020115 - Attachment mime type: application/octet-stream → text/plain

Comment 20

8 months ago
Neil, I assume this review request isn't urgent since there are other parts still waiting, I may be able to do it next week.
Assignee

Comment 21

8 months ago
Another issue that came up here is that some bindings implement nsIObserver that will now be custom elements, such as the newly added search-one-off buttons (in bug 1493536). However, with the patches here, Element doesn't use the JS-implemented QueryInterface, so for example:
   Services.prefs.addObserver("browser.search.hiddenOneOffs", this, true);
doesn't work, as 'this' isn't an observer (and doesn't work with or without a JS-implemented QueryInterface added in customElements.js

The simplest solution here is to use a helper object that implements nsIObserver. Similarly, I think we may need the same for the autocomplete interfaces, but I am going to leave them as is for now until autocomplete.xml is worked on.
(In reply to Neil Deakin from comment #21)
> The simplest solution here is to use a helper object that implements
> nsIObserver. Similarly, I think we may need the same for the autocomplete
> interfaces, but I am going to leave them as is for now until
> autocomplete.xml is worked on.

Using a helper object for nsIObserver works for me, and it's something we've done before. I believe we can generally even convert existing XBL bindings to do that before migration to Custom Element if necessary.
Assignee

Updated

8 months ago
Attachment #9018131 - Flags: review?(peterv)
Assignee

Comment 24

8 months ago
Comment on attachment 9020109 [details] [diff] [review]
Part 2 - add helpers to Element to get nsIDOMXUL* and nsIBrowser interfaces mplented by custom elements

As per bz's suggestion.
Attachment #9020109 - Flags: review?(peterv)
Assignee

Updated

8 months ago
Attachment #9020118 - Flags: review?(peterv)
Assignee

Comment 26

8 months ago
Comment on attachment 9020115 [details] [diff] [review]
Part 5 - Use Element helper methods instead of QueryInterface to get nsIBrowser

># HG changeset patch
># Parent  5110138a20de10d64d3c112c973a24a3ad4a59eb
>Bug 1492326, revert some of bug 1478372, so that callers need to get the custom interface from a custom element without using QueryInterface.
>
>diff --git a/dom/base/nsContentUtils.cpp b/dom/base/nsContentUtils.cpp
>--- a/dom/base/nsContentUtils.cpp
>+++ b/dom/base/nsContentUtils.cpp
>@@ -10617,17 +10617,21 @@ nsContentUtils::CreateJSValueFromSequenc
>   return NS_OK;
> }
> 
> /* static */
> bool
> nsContentUtils::ShouldBlockReservedKeys(WidgetKeyboardEvent* aKeyEvent)
> {
>   nsCOMPtr<nsIPrincipal> principal;
>-  nsCOMPtr<nsIBrowser> targetBrowser = do_QueryInterface(aKeyEvent->mOriginalTarget);
>+  nsCOMPtr<Element> targetElement = do_QueryInterface(aKeyEvent->mOriginalTarget);
>+  nsCOMPtr<nsIBrowser> targetBrowser;
>+  if (targetElement) {
>+    targetBrowser = targetElement->AsBrowser();
>+  }
>   bool isRemoteBrowser = false;
>   if (targetBrowser) {
>     targetBrowser->GetIsRemoteBrowser(&isRemoteBrowser);
>   }
> 
>   if (isRemoteBrowser) {
>     targetBrowser->GetContentPrincipal(getter_AddRefs(principal));
>   }
>diff --git a/dom/base/nsFrameLoader.cpp b/dom/base/nsFrameLoader.cpp
>--- a/dom/base/nsFrameLoader.cpp
>+++ b/dom/base/nsFrameLoader.cpp
>@@ -2459,17 +2459,17 @@ nsFrameLoader::SetClampScrollPosition(bo
> }
> 
> static
> Tuple<ContentParent*, TabParent*>
> GetContentParent(Element* aBrowser)
> {
>   using ReturnTuple = Tuple<ContentParent*, TabParent*>;
> 
>-  nsCOMPtr<nsIBrowser> browser = do_QueryInterface(aBrowser);
>+  nsCOMPtr<nsIBrowser> browser = aBrowser ? aBrowser->AsBrowser() : nullptr;
>   if (!browser) {
>     return ReturnTuple(nullptr, nullptr);
>   }
> 
>   RefPtr<nsFrameLoader> otherLoader;
>   browser->GetSameProcessAsFrameLoader(getter_AddRefs(otherLoader));
>   if (!otherLoader) {
>     return ReturnTuple(nullptr, nullptr);
>diff --git a/dom/ipc/TabParent.cpp b/dom/ipc/TabParent.cpp
>--- a/dom/ipc/TabParent.cpp
>+++ b/dom/ipc/TabParent.cpp
>@@ -542,17 +542,17 @@ TabParent::RecvSizeShellTo(const uint32_
>   xulWin->SizeShellToWithLimit(width, height, aShellItemWidth, aShellItemHeight);
> 
>   return IPC_OK();
> }
> 
> mozilla::ipc::IPCResult
> TabParent::RecvDropLinks(nsTArray<nsString>&& aLinks)
> {
>-  nsCOMPtr<nsIBrowser> browser = do_QueryInterface(mFrameElement);
>+  nsCOMPtr<nsIBrowser> browser = mFrameElement ? mFrameElement->AsBrowser() : nullptr;
>   if (browser) {
>     // Verify that links have not been modified by the child. If links have
>     // not been modified then it's safe to load those links using the
>     // SystemPrincipal. If they have been modified by web content, then
>     // we use a NullPrincipal which still allows to load web links.
>     bool loadUsingSystemPrincipal = true;
>     if (aLinks.Length() != mVerifyDropLinks.Length()) {
>       loadUsingSystemPrincipal = false;
>@@ -2040,17 +2040,17 @@ TabParent::RecvRequestFocus(const bool& 
>   return IPC_OK();
> }
> 
> mozilla::ipc::IPCResult
> TabParent::RecvEnableDisableCommands(const nsString& aAction,
>                                      nsTArray<nsCString>&& aEnabledCommands,
>                                      nsTArray<nsCString>&& aDisabledCommands)
> {
>-  nsCOMPtr<nsIBrowser> browser = do_QueryInterface(mFrameElement);
>+  nsCOMPtr<nsIBrowser> browser = mFrameElement ? mFrameElement->AsBrowser() : nullptr;
>   bool isRemoteBrowser = false;
>   if (browser) {
>     browser->GetIsRemoteBrowser(&isRemoteBrowser);
>   }
>   if (isRemoteBrowser) {
>     UniquePtr<const char*[]> enabledCommands, disabledCommands;
> 
>     if (aEnabledCommands.Length()) {
>@@ -3602,17 +3602,17 @@ TabParent::RecvLookUpDictionary(const ns
>   widget->LookUpDictionary(aText, aFontRangeArray, aIsVertical,
>                            aPoint - GetChildProcessOffset());
>   return IPC_OK();
> }
> 
> mozilla::ipc::IPCResult
> TabParent::RecvShowCanvasPermissionPrompt(const nsCString& aFirstPartyURI)
> {
>-  nsCOMPtr<nsIBrowser> browser = do_QueryInterface(mFrameElement);
>+  nsCOMPtr<nsIBrowser> browser = mFrameElement ? mFrameElement->AsBrowser() : nullptr;
>   if (!browser) {
>     // If the tab is being closed, the browser may not be available.
>     // In this case we can ignore the request.
>     return IPC_OK();
>   }
>   nsCOMPtr<nsIObserverService> os = services::GetObserverService();
>   if (!os) {
>     return IPC_FAIL_NO_REASON(this);
Attachment #9020115 - Attachment is obsolete: true
Assignee

Updated

8 months ago
Attachment #9020114 - Flags: review?(paolo.mozmail)
Assignee

Updated

8 months ago
Depends on: 1505814
Assignee

Comment 27

8 months ago
Comment on attachment 9022682 [details] [diff] [review]
Part 8 - switch to helper object for nsIObserver implementation

Moved this part to bug 1505814.
Attachment #9022682 - Attachment is obsolete: true
Depends on: 1506261
No longer depends on: 1505814
Comment on attachment 9018131 [details] [diff] [review]
Part 1 - change return values of nsIDOMXUL* interfaces to return Elements

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

::: accessible/xul/XULSelectControlAccessible.cpp
@@ +131,4 @@
>      multiSelectControl->AddItemToSelection(itemElm);
> +  } else {
> +    nsCOMPtr<Element> element = do_QueryInterface(item->GetContent());
> +    mSelectControl->SetSelectedItem(element);

Just pass in |item->Elm()| instead?

@@ +239,5 @@
>  {
>    if (!mSelectControl)
>      return;
>  
> +  nsCOMPtr<Element> itemElm = do_QueryInterface(aItem->GetContent());

|aItem->Elm()|
Attachment #9018131 - Flags: review?(peterv) → review+
Attachment #9020109 - Flags: review?(peterv) → review+
Comment on attachment 9020118 [details] [diff] [review]
Part 7 - remove call to CallGetCustomInterface from nsXULElement::QueryInterface once all needed callers are removed

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

::: dom/base/CustomElementRegistry.cpp
@@ +1315,5 @@
> +// Otherwise, check if the element supports the interface directly, and just use that.
> + nsCOMPtr<nsISupports> supports;
> + if (NS_SUCCEEDED(aElement->QueryInterface(aIID, getter_AddRefs(supports)))) {
> +   return supports.forget();
> + }

I don't understand why we need this part, could you explain?
Flags: needinfo?(enndeakin)
Assignee

Comment 30

7 months ago
> > +// Otherwise, check if the element supports the interface directly, and just use that.
> > + nsCOMPtr<nsISupports> supports;
> > + if (NS_SUCCEEDED(aElement->QueryInterface(aIID, getter_AddRefs(supports)))) {
> > +   return supports.forget();
> > + }
> 
> I don't understand why we need this part, could you explain?

So that it falls back to checking interfaces implemented in the XBL binding via the 'implements' attribute.  

This patch just reverts the removal of the block of code in bug 1478372.
Flags: needinfo?(enndeakin)
Attachment #9020118 - Flags: review?(peterv) → review+

Comment 31

7 months ago
Comment on attachment 9020114 [details] [diff] [review]
Part 4 - Use Element helper methods in dom/layout instead of QueryInterface

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

::: layout/xul/nsButtonBoxFrame.cpp
@@ +132,5 @@
>          break;
>        }
>        if (NS_VK_RETURN == keyEvent->mKeyCode) {
> +        if (mContent->IsXULElement(nsGkAtoms::button) ||
> +            mContent->IsXULElement(nsGkAtoms::toolbarbutton)) {

There are more elements implementing nsIDOMXULButtonElement, so I'm not sure this is correct.
Attachment #9020114 - Flags: review?(paolo.mozmail)

Updated

7 months ago
Attachment #9020117 - Flags: review?(paolo.mozmail)
Assignee

Comment 32

7 months ago
Attachment #9020114 - Attachment is obsolete: true
Attachment #9029804 - Flags: review?(paolo.mozmail)
Assignee

Comment 33

7 months ago
Attachment #9020117 - Attachment is obsolete: true
Assignee

Updated

7 months ago
Attachment #9029807 - Flags: review?(paolo.mozmail)
Assignee

Updated

7 months ago
Attachment #9023343 - Flags: review?(paolo.mozmail)

Comment 34

7 months ago
Comment on attachment 9029804 [details] [diff] [review]
Part 4 - Use Element helper methods in dom/layout instead of QueryInterface

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

r+ with the below addressed.

::: layout/xul/nsMenuPopupFrame.cpp
@@ +1106,5 @@
>  
>  nsIFrame* nsMenuPopupFrame::GetSelectedItemForAlignment() {
> +  if (!mAnchorContent) {
> +    return nullptr;
> +  }

This doesn't look coherent with the code below. If this early return is intentional, the rest of the function needs to be changed as well. Otherwise, just remove it.
Attachment #9029804 - Flags: review?(paolo.mozmail) → review+

Updated

7 months ago
Attachment #9023343 - Flags: review?(paolo.mozmail) → review+

Comment 35

7 months ago
Comment on attachment 9029807 [details] [diff] [review]
Part 6 - Use tagnames instead of nsIDOMXUL* interfaces in mobile/toolkit

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

::: toolkit/content/tests/chrome/test_custom_element_base.xul
@@ +194,5 @@
>      customElements.define("extendedelement", ExtendedElement);
>      const extendedInstance = document.createXULElement("extendedelement");
> +    ok(extendedInstance.QueryInterface(Ci.nsIDOMXULSelectControlElement), "interface applied");
> +    // XXXndeakin I would expect that ChromeUtils.generateQI would just handle inheritance?
> +    // ok(extendedInstance.QueryInterface(Ci.nsIDOMXULControlElement), "inherited interface applied");

With generateQI you have to specify all the interfaces explicitly, even if one is the parent of the other. You can't assume that QueryInterface works when you pass a base interface as its argument, even if it works for an interface derived from it, if this is what you're asking.

::: toolkit/content/widgets/richlistbox.xml
@@ +582,2 @@
>              let children = Array.from(this.children)
> +                                .filter(node => node.nodeType == 1 &&

Is the nodeType check still required when iterating "children"?
Attachment #9029807 - Flags: review?(paolo.mozmail) → review+
Assignee

Comment 36

7 months ago
> ::: toolkit/content/tests/chrome/test_custom_element_base.xul
> @@ +194,5 @@
> >      customElements.define("extendedelement", ExtendedElement);
> >      const extendedInstance = document.createXULElement("extendedelement");
> > +    ok(extendedInstance.QueryInterface(Ci.nsIDOMXULSelectControlElement), "interface applied");
> > +    // XXXndeakin I would expect that ChromeUtils.generateQI would just handle inheritance?
> > +    // ok(extendedInstance.QueryInterface(Ci.nsIDOMXULControlElement), "inherited interface applied");
> 
> With generateQI you have to specify all the interfaces explicitly, even if
> one is the parent of the other. You can't assume that QueryInterface works
> when you pass a base interface as its argument, even if it works for an
> interface derived from it, if this is what you're asking.

Brian, do you think this will be a problem? Should I just disable this part of the test?
Flags: needinfo?(bgrinstead)
(In reply to Neil Deakin from comment #36)
> > ::: toolkit/content/tests/chrome/test_custom_element_base.xul
> > @@ +194,5 @@
> > >      customElements.define("extendedelement", ExtendedElement);
> > >      const extendedInstance = document.createXULElement("extendedelement");
> > > +    ok(extendedInstance.QueryInterface(Ci.nsIDOMXULSelectControlElement), "interface applied");
> > > +    // XXXndeakin I would expect that ChromeUtils.generateQI would just handle inheritance?
> > > +    // ok(extendedInstance.QueryInterface(Ci.nsIDOMXULControlElement), "inherited interface applied");
> > 
> > With generateQI you have to specify all the interfaces explicitly, even if
> > one is the parent of the other. You can't assume that QueryInterface works
> > when you pass a base interface as its argument, even if it works for an
> > interface derived from it, if this is what you're asking.
> 
> Brian, do you think this will be a problem? Should I just disable this part
> of the test?

The current implementation of implementCustomInterface explicitly supports inheriting interfaces: https://searchfox.org/mozilla-central/rev/ff46b36ac2ebb243ec95fdab01340391963d62e5/toolkit/content/customElements.js#216-217. We could stop doing that, but then any existing and future consumer of `MozElements.BaseControl` would have to manually implement `Ci.nsIDOMXULControlElement` (as an example).

Would it be possible to keep track of which interfaces base classes implement and pass them all into generateQI? I guess if not I'd be OK with removing that feature, as long as we audit all existing consumers to make sure we don't un-implement something on accident.
Flags: needinfo?(bgrinstead)
Assignee

Comment 38

7 months ago
This is an updated version that addresses comments and mostly just updates customElements.js to handle inheritance.
Attachment #9029807 - Attachment is obsolete: true
Attachment #9030053 - Flags: review?(paolo.mozmail)

Comment 39

7 months ago
Comment on attachment 9030053 [details] [diff] [review]
Part 6 - Use tagnames instead of nsIDOMXUL* interfaces in mobile/toolkit

Thanks!
Attachment #9030053 - Flags: review?(paolo.mozmail) → review+

Comment 40

7 months ago
Pushed by neil@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/8e8ca5becf68
change methods of nsIDOMXUL* interfaces so that they return Elements to reduce usages as most callers want the return values as elements, r=peterv
https://hg.mozilla.org/integration/mozilla-inbound/rev/f69b2af6280a
add helpers to Element to get nsIDOMXUL* and nsIBrowser interfaces implented by custom elements, r=peterv
https://hg.mozilla.org/integration/mozilla-inbound/rev/8837771de85a
use Element helper methods in accessibility instead of QueryInterface to get interface implementations that might be implemented by custom elements, r=surkov
https://hg.mozilla.org/integration/mozilla-inbound/rev/ce5c9d031921
use Element helper methods in dom/layout instead of QueryInterface to get interface implementations that might be implemented by custom elements, r=paolo
https://hg.mozilla.org/integration/mozilla-inbound/rev/d395c2dbcbb0
don't use QueryInterface to get nsIBrowser as it may be implemented in a custom element, r=paolo
https://hg.mozilla.org/integration/mozilla-inbound/rev/8f0db5ec605d
revert some of bug 1478372, so that callers need to get the custom interface from a custom element without using QueryInterface, r=peterv
https://hg.mozilla.org/integration/mozilla-inbound/rev/0c7a54d4cc42
don't use 'instanceof nsiDOMXUL*' in toolkit and mobile files, r=paolo
Depends on: 1513332
Depends on: 1513469
Duplicate of this bug: 1496067

Updated

6 months ago
Depends on: 1518054
Blocks: 1517101

Updated

19 days ago
Type: enhancement → task
You need to log in before you can comment on or make changes to this bug.