1166706 - Clicking the empty space of Pocket's confirmation panel results in an error thrown by CustomizableUI.jsm:1536:10

Status: RESOLVED FIXED

(Firefox :: Pocket, defect, P5)

Description

[Andrei Vaida [:avaida]]

Reproducible on:
Beta 38.0.5b3-build1 (20150518141916)

Affected platforms:
Window 7 (x64), Mac OS X 10.9.5, Ubuntu 14.04 (x64)

Preconditions:
* user is logged into Pocket

Steps to reproduce:
1. Open a random page, e.g. https://www.mozilla.org/en-US/mission/
2. Click the Pocket toolbar button.
3. Click anywhere on the empty space available in the confirmation panel displayed by Pocket.

Expected result:
There are no errors thrown for this action.

Actual result:
Clicking the empty space from the confirmation panel results in CustomizableUI.jsm:1536:10 throwing the following error in the Browser Console:
> TypeError: target.getAttribute is not a function

Comment 1

[Justin Dolske [:Dolske]]

I get this too, error is in the conditional in _isOnInteractiveElement():

      ...
      // Break out of the loop immediately for disabled items, as we need to
      // keep the menu open in that case.
      if (target.getAttribute("disabled") == "true") {
        return true;
      }
      ...

Gijs, is this concerning or not? (I sorta suspect this may be unique to the panel contents being an iframe, event.target can be weird in such cases.)

Comment 2

[:Gijs (he/him)]

Attachment: MozReview Request: bz://1166706/Gijs

/r/9357 - Bug 1166706 - deal with iframes in panels, r?dolske

Pull down this commit:

hg pull -r 944a68ba1fb020f80da2914cb6108a4813a89a76 https://reviewboard-hg.mozilla.org/gecko/

Comment 3

[:Gijs (he/him)]

Comment on attachment 8610673 [details]
MozReview Request: bz://1166706/Gijs

/r/9357 - Bug 1166706 - deal with iframes in panels, r?dolske

Pull down this commit:

hg pull -r 944a68ba1fb020f80da2914cb6108a4813a89a76 https://reviewboard-hg.mozilla.org/gecko/

Comment 4

[Justin Dolske [:Dolske]]

Comment on attachment 8610673 [details]
MozReview Request: bz://1166706/Gijs

https://reviewboard.mozilla.org/r/9355/#review8095

::: browser/components/customizableui/CustomizableUI.jsm:1514
(Diff revision 1)
> +          break;

Shouldn't this just always bail when we hit a DOCUMENT_NODE?

Perhaps I'm not being creative, but I'm having a hard time imagining cases where someone would put an <iframe> in the other things we're checking for in this function?

Comment 5

[:Gijs (he/him)]

Comment on attachment 8610673 [details]
MozReview Request: bz://1166706/Gijs

(In reply to Justin Dolske [:Dolske] from comment #4)
> Comment on attachment 8610673 [details]
> MozReview Request: bz://1166706/Gijs
> 
> https://reviewboard.mozilla.org/r/9355/#review8095
> 
> ::: browser/components/customizableui/CustomizableUI.jsm:1514
> (Diff revision 1)
> > +          break;
> 
> Shouldn't this just always bail when we hit a DOCUMENT_NODE?
> 
> Perhaps I'm not being creative, but I'm having a hard time imagining cases
> where someone would put an <iframe> in the other things we're checking for
> in this function?

While I agree with the sentiment, the SDK does exactly this (iframes in toolbarbuttons for "widget").

Comment 6

[Justin Dolske [:Dolske]]

https://reviewboard.mozilla.org/r/9355/#review8577

Comment 7

[Justin Dolske [:Dolske]]

Comment on attachment 8610673 [details]
MozReview Request: bz://1166706/Gijs

*sigh* I can't tell how to, y'know, actually have ReviewBoard mark this as r+.

Comment 8

[Pulsebot]

https://hg.mozilla.org/integration/fx-team/rev/d975c1a22c75

Comment 9

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/d975c1a22c75

Comment 11

[:Gijs (he/him)]

Attachment: MozReview Request: Bug 1166706 - deal with iframes in panels, r?dolske