1305148 - panel.resize throws if called when panel is hidden

Status: RESOLVED FIXED

(Add-on SDK Graveyard :: General, defect)

Description

[Harsha A]

User Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10_11_6) AppleWebKit/602.1.50 (KHTML, like Gecko) Version/10.0 Safari/602.1.50

Steps to reproduce:

Install an extension that uses the Panel API (https://developer.mozilla.org/en-US/Add-ons/SDK/High-Level_APIs/panel) that is attached to a button. 

Clicking on the button should bring down your Panel UI. 


Actual results:

I keep getting an exception. 

nortonsecurity:TypeError: panel.firstChild is null
Stack trace:
resize@resource://gre/modules/commonjs/toolkit/loader.js -> resource://gre/modules/commonjs/sdk/panel/utils.js:109:3
resize@resource://gre/modules/commonjs/toolkit/loader.js -> resource://gre/modules/commonjs/sdk/panel.js:317:5
setUIStatusForTab@resource://gre/modules/commonjs/toolkit/loader.js -> resource://nortonsecurity-at-symantec-dot-com/lib/main.js:12:380
handleClick@resource://gre/modules/commonjs/toolkit/loader.js -> resource://nortonsecurity-at-symantec-dot-com/lib/main.js:23:373
emitOnObject@resource://gre/modules/commonjs/toolkit/loader.js -> resource://gre/modules/commonjs/sdk/event/core.js:112:9
emit@resource://gre/modules/commonjs/toolkit/loader.js -> resource://gre/modules/commonjs/sdk/event/core.js:89:38
@resource://gre/modules/commonjs/toolkit/loader.js -> resource://gre/modules/commonjs/sdk/ui/button/action.js:101:3
emitOnObject@resource://gre/modules/commonjs/toolkit/loader.js -> resource://gre/modules/commonjs/sdk/event/core.js:112:9
emit@resource://gre/modules/commonjs/toolkit/loader.js -> resource://gre/modules/commonjs/sdk/event/core.js:89:38
receive@resource://gre/modules/commonjs/toolkit/loader.js -> resource://gre/modules/commonjs/sdk/event/utils.js:115:5
transform/next@resource://gre/modules/commonjs/toolkit/loader.js -> resource://gre/modules/commonjs/sdk/event/utils.js:37:24
filter/<@resource://gre/modules/commonjs/toolkit/loader.js -> resource://gre/modules/commonjs/sdk/event/utils.js:54:7
transform/<@resource://gre/modules/commonjs/toolkit/loader.js -> resource://gre/modules/commonjs/sdk/event/utils.js:44:29
emitOnObject@resource://gre/modules/commonjs/toolkit/loader.js -> resource://gre/modules/commonjs/sdk/event/core.js:112:9
emit@resource://gre/modules/commonjs/toolkit/loader.js -> resource://gre/modules/commonjs/sdk/event/core.js:89:38
receive@resource://gre/modules/commonjs/toolkit/loader.js -> resource://gre/modules/commonjs/sdk/event/utils.js:115:5
transform/next@resource://gre/modules/commonjs/toolkit/loader.js -> resource://gre/modules/commonjs/sdk/event/utils.js:37:24
filter/<@resource://gre/modules/commonjs/toolkit/loader.js -> resource://gre/modules/commonjs/sdk/event/utils.js:54:7
transform/<@resource://gre/modules/commonjs/toolkit/loader.js -> resource://gre/modules/commonjs/sdk/event/utils.js:44:29
emitOnObject@resource://gre/modules/commonjs/toolkit/loader.js -> resource://gre/modules/commonjs/sdk/event/core.js:112:9
emit@resource://gre/modules/commonjs/toolkit/loader.js -> resource://gre/modules/commonjs/sdk/event/core.js:89:38
create/<.onBuild/<@resource://gre/modules/commonjs/toolkit/loader.js -> resource://gre/modules/commonjs/sdk/ui/button/view.js:155:11
EventListener.handleEvent*EventTargetInterposition.methods.addEventListener@resource://gre/modules/RemoteAddonsParent.jsm:652:5
AddonInterpositionService.prototype.interposeProperty/desc.value@resource://gre/components/multiprocessShims.js:160:52
create/<.onBuild@resource://gre/modules/commonjs/toolkit/loader.js -> resource://gre/modules/commonjs/sdk/ui/button/view.js:153:7
CustomizableUIInternal.wrapWidgetEventHandler/aWidget[aEventName]@resource://app/modules/CustomizableUI.jsm:2420:16
CustomizableUIInternal.buildWidget@resource://app/modules/CustomizableUI.jsm:1342:16
CustomizableUIInternal.getWidgetNode@resource://app/modules/CustomizableUI.jsm:895:16
CustomizableUIInternal.insertNodeInWindow@resource://app/modules/CustomizableUI.jsm:1148:26
CustomizableUIInternal.insertNode@resource://app/modules/CustomizableUI.jsm:1134:7
CustomizableUIInternal.onWidgetAdded@resource://app/modules/CustomizableUI.jsm:940:5
CustomizableUIInternal.notifyListeners@resource://app/modules/CustomizableUI.jsm:2119:11
CustomizableUIInternal.createWidget@resource://app/modules/CustomizableUI.jsm:2229:9
this.CustomizableUI.createWidget@resource://app/modules/CustomizableUI.jsm:3276:7
create@resource://gre/modules/commonjs/toolkit/loader.js -> resource://gre/modules/commonjs/sdk/ui/button/view.js:122:3
setup@resource://gre/modules/commonjs/toolkit/loader.js -> resource://gre/modules/commonjs/sdk/ui/button/action.js:60:5
@resource://gre/modules/commonjs/toolkit/loader.js -> resource://gre/modules/commonjs/sdk/core/disposable.js:45:56
dispatch@resource://gre/modules/commonjs/toolkit/loader.js -> resource://gre/modules/commonjs/method/core.js:119:12
Disposable<.initialize@resource://gre/modules/commonjs/toolkit/loader.js -> resource://gre/modules/commonjs/sdk/core/disposable.js:135:17
constructor@resource://gre/modules/commonjs/toolkit/loader.js -> resource://gre/modules/commonjs/sdk/core/heritage.js:146:23
@resource://gre/modules/commonjs/toolkit/loader.js -> resource://nortonsecurity-at-symantec-dot-com/lib/main.js:7:168
evaluate@resource://gre/modules/commonjs/toolkit/loader.js:279:19
load@resource://gre/modules/commonjs/toolkit/loader.js:331:5
main@resource://gre/modules/commonjs/toolkit/loader.js:786:10
run@resource://gre/modules/commonjs/toolkit/loader.js -> resource://gre/modules/commonjs/sdk/addon/runner.js:147:19
startup/</<@resource://gre/modules/commonjs/toolkit/loader.js -> resource://gre/modules/commonjs/sdk/addon/runner.js:87:9
Handler.prototype.process@resource://gre/modules/Promise.jsm -> resource://gre/modules/Promise-backend.js:937:23
this.PromiseWalker.walkerLoop@resource://gre/modules/Promise.jsm -> resource://gre/modules/Promise-backend.js:816:7
Promise*this.PromiseWalker.scheduleWalkerLoop@resource://gre/modules/Promise.jsm -> resource://gre/modules/Promise-backend.js:747:11
this.PromiseWalker.schedulePromise@resource://gre/modules/Promise.jsm -> resource://gre/modules/Promise-backend.js:779:7
this.PromiseWalker.completePromise@resource://gre/modules/Promise.jsm -> resource://gre/modules/Promise-backend.js:714:7
handler@resource://gre/modules/commonjs/toolkit/loader.js -> resource://gre/modules/commonjs/sdk/addon/window.js:56:3
EventListener.handleEvent*EventTargetInterposition.methods.addEventListener@resource://gre/modules/RemoteAddonsParent.jsm:652:5
AddonInterpositionService.prototype.interposeProperty/desc.value@resource://gre/components/multiprocessShims.js:160:52
@resource://gre/modules/commonjs/toolkit/loader.js -> resource://gre/modules/commonjs/sdk/addon/window.js:54:1
evaluate@resource://gre/modules/commonjs/toolkit/loader.js:279:19
load@resource://gre/modules/commonjs/toolkit/loader.js:331:5
_require@resource://gre/modules/commonjs/toolkit/loader.js:671:16
require@resource://gre/modules/commonjs/toolkit/loader.js:625:12
startup/<@resource://gre/modules/commonjs/toolkit/loader.js -> resource://gre/modules/commonjs/sdk/addon/runner.js:72:21
Handler.prototype.process@resource://gre/modules/Promise.jsm -> resource://gre/modules/Promise-backend.js:937:23
this.PromiseWalker.walkerLoop@resource://gre/modules/Promise.jsm -> resource://gre/modules/Promise-backend.js:816:7
Promise*this.PromiseWalker.scheduleWalkerLoop@resource://gre/modules/Promise.jsm -> resource://gre/modules/Promise-backend.js:747:11
this.PromiseWalker.schedulePromise@resource://gre/modules/Promise.jsm -> resource://gre/modules/Promise-backend.js:779:7
this.PromiseWalker.completePromise@resource://gre/modules/Promise.jsm -> resource://gre/modules/Promise-backend.js:714:7
listener/<@resource://gre/modules/sdk/system/Startup.js:52:46
  core.js:106

        


Expected results:

It should have pulled down our UI.

Comment 1

[Tooru Fujisawa [:arai]]

can you provide a testcase?

Comment 2

[Harsha A]

@Tooru: You can download our xpi at http://macplugin.norton.com/NFM/7/Firefox/[e-mail]

We use the Panel API mentioned https://developer.mozilla.org/en-US/Add-ons/SDK/High-Level_APIs/panel. 

In the Nightly build, once I install the extension and click on the toolbar button, we should see our product UI show up. However in the Nightly build it does not. It throws an exception. 

This works fine in Release and Beta versions of Firefox. 

I hope this helps. 

Thanks,
Harsha

Comment 3

[Tooru Fujisawa [:arai]]

thanks.

here's regression range:
https://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?fromchange=e3fa462347c8c863e310d4e0e45e470c9775bdcd&tochange=1430290455ed94109e973fd17c7f62865e1695c9

seems like a regression from bug 1294199.

kmag, can you have a look?

Comment 4

[Kris Maglione [:kmag]]

Attachment: Bug 1305148: Ignore panel.resize when the panel isn't visible.

Review commit: https://reviewboard.mozilla.org/r/81834/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/81834/

Comment 5

[Gabor Krizsanits (INACTIVE)]

Comment on attachment 8795885 [details]
Bug 1305148: Ignore panel.resize when the panel isn't visible.

https://reviewboard.mozilla.org/r/81834/#review80952

::: addon-sdk/source/test/test-panel.js:339
(Diff revision 1)
>      assert.fail('error was emitted:' + e.message + '\n' + e.stack);
>    });
>    panel.show();
>  };
>  
> -exports["test Anchor And Arrow"] = function(assert, done) {
> +exports["test Anchor And Arrow"] = function*(assert, done) {

Is this intentional?

Comment 6

[Kris Maglione [:kmag]]

Comment on attachment 8795885 [details]
Bug 1305148: Ignore panel.resize when the panel isn't visible.

https://reviewboard.mozilla.org/r/81834/#review80952

> Is this intentional?

Yeah. All of the other generator functions in that file had already been changed, and ESLint wouldn't run on the file until that one was fixed too.

Comment 7

[Kris Maglione [:kmag]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/e517fba7718a4ffd0c7dbfc1d133da7dc39e71f3
Bug 1305148: Ignore panel.resize when the panel isn't visible. r=gabor

Comment 8

[Kris Maglione [:kmag]]

Comment on attachment 8795885 [details]
Bug 1305148: Ignore panel.resize when the panel isn't visible.

Approval Request Comment
[Feature/regressing bug #]: Bug 1294199
[User impact if declined]: This bug causes certain SDK add-ons to malfunction if they call panel.resize() when the panel is not visible. While prior to bug 1294199, calling that function while the panel was invisible had no effect, it currently throws an error, which can prevent necessary code from running in response to user actions.
[Describe test coverage new/current, TreeHerder]: The panel code which this effects is generally well-tested, and new test coverage has been added for this particular issue.
[Risks and why]: Very low. This patch simply adds a conditional which prevents us attempting to resize an element when it doesn't exist. Prior to bug 1294199, the changes were a no-op anyway, so there should be no significant change in behavior.
[String/UUID change made/needed]: None.

Comment 9

[Ritu Kothari (:ritu) (Inactive, please n-i to RyanVM, jcristau, or pascal)]

Hello Harsha, could you please verify this issue is fixed as expected on a latest Nightly build? Thanks!

Comment 10

[Phil Ringnalda (:philor)]

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

Comment 11

[Ritu Kothari (:ritu) (Inactive, please n-i to RyanVM, jcristau, or pascal)]

Comment on attachment 8795885 [details]
Bug 1305148: Ignore panel.resize when the panel isn't visible.

Fixes a recent regression, Aurora51+, Beta50+

Comment 12

[Harsha A]

I checked the latest nightly build. It is working again. Thank you.

Comment 13

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

https://hg.mozilla.org/releases/mozilla-aurora/rev/1fce6a4ff9e3

Comment 14

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

https://hg.mozilla.org/releases/mozilla-beta/rev/9d64a6fe3f45