Closed Bug 1305148 Opened 3 years ago Closed 3 years ago

panel.resize throws if called when panel is hidden

Categories

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

defect
Not set
major

Tracking

(firefox50 fixed, firefox51 fixed, firefox52 fixed)

RESOLVED FIXED
mozilla52
Tracking Status
firefox50 --- fixed
firefox51 --- fixed
firefox52 --- fixed

People

(Reporter: sriharsha.angara, Assigned: kmag, NeedInfo)

References

Details

(Keywords: regression)

Attachments

(1 file)

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.
Summary: Panel API not loading → Panel API not loading UI
Severity: normal → blocker
can you provide a testcase?
Severity: blocker → major
Component: Untriaged → General
Flags: needinfo?(sriharsha.angara)
Product: Firefox → Add-on SDK
Version: 52 Branch → unspecified
@Tooru: You can download our xpi at http://macplugin.norton.com/NFM/7/Firefox/nortonsecurity@symantec.com.xpi

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
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?
Blocks: 1294199
Status: UNCONFIRMED → NEW
Ever confirmed: true
Flags: needinfo?(sriharsha.angara) → needinfo?(kmaglione+bmo)
Keywords: regression
Assignee: nobody → kmaglione+bmo
Flags: needinfo?(kmaglione+bmo)
Summary: Panel API not loading UI → panel.resize throws if called when panel is hidden
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?
Attachment #8795885 - Flags: review?(gkrizsanits) → review+
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 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.
Attachment #8795885 - Flags: approval-mozilla-beta?
Attachment #8795885 - Flags: approval-mozilla-aurora?
Hello Harsha, could you please verify this issue is fixed as expected on a latest Nightly build? Thanks!
Flags: needinfo?(sriharsha.angara)
https://hg.mozilla.org/mozilla-central/rev/e517fba7718a
Status: NEW → RESOLVED
Closed: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla52
Comment on attachment 8795885 [details]
Bug 1305148: Ignore panel.resize when the panel isn't visible.

Fixes a recent regression, Aurora51+, Beta50+
Attachment #8795885 - Flags: approval-mozilla-beta?
Attachment #8795885 - Flags: approval-mozilla-beta+
Attachment #8795885 - Flags: approval-mozilla-aurora?
Attachment #8795885 - Flags: approval-mozilla-aurora+
I checked the latest nightly build. It is working again. Thank you.
You need to log in before you can comment on or make changes to this bug.