Missing check in onWindowLoaded callback if a new window has a getBrowser() method

RESOLVED FIXED

Status

Testing Graveyard
Mozmill
RESOLVED FIXED
5 years ago
a year ago

People

(Reporter: whimboo, Assigned: whimboo)

Tracking

({regression})

Details

(Whiteboard: [mozmill-2.0+])

Attachments

(1 attachment)

(Assignee)

Description

5 years ago
Looks like we do not handle that failure correctly yet. You can reproduce by trying to open the certificate details for https://wiki.mozilla.org/

CRITICAL | Framework Failure | {
  "message": "[JavaScript Error: \"TypeError: aWindow.getBrowser is not a function\" {file: \"resource://mozmill/driver/mozmill.js\" line: 383}]"
}

I assume it's simply a missing check. We should fix that for 2.0.
(Assignee)

Comment 1

5 years ago
This is a regression from bug 696494 which we would have to fix.
Blocks: 696494
Keywords: regression
(Assignee)

Updated

5 years ago
Summary: Opening a window without a gBrowser object kills Firefox → Missing check in onWindowLoaded callback if a new window has a getBrowser() method
(Assignee)

Comment 2

5 years ago
Created attachment 663329 [details]
Patch v1

Pointer to Github pull-request
(Assignee)

Comment 3

5 years ago
Comment on attachment 663329 [details]
Patch v1

Simple fix to ensure we check first that the window really has a getBrowser() method attached to it.
Attachment #663329 - Attachment description: Pointer to Github pull request: https://github.com/mozilla/mozmill/pull/102 → Patch v1
Attachment #663329 - Flags: review?(ahalberstadt)
Comment on attachment 663329 [details]
Patch v1

r+ with one nit:

Please move the aWindow.getBrowser() to a separate if statement so we only need to call it once.
Attachment #663329 - Flags: review?(ahalberstadt) → review+
(Assignee)

Comment 5

5 years ago
(In reply to Andrew Halberstadt [:ahal] from comment #4)
> Please move the aWindow.getBrowser() to a separate if statement so we only
> need to call it once.

Is this really necessary? It makes the code more complex while it only returns the gBrowser property.
(Assignee)

Comment 6

5 years ago
Ok, so the following code does what you want in a single command:

var browser = ("getBrowser" in aWindow) ? aWindow.getBrowser() : null;

Pushed to master:
https://github.com/mozilla/mozmill/commit/fcc4a97b3d047a266c62f0a101562c1b58fcfc41
Assignee: nobody → hskupin
Status: NEW → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
Whiteboard: [mentor=whimboo][lang=js][mozmill-2.0?] → [mozmill-2.0+]
Product: Testing → Testing Graveyard
You need to log in before you can comment on or make changes to this bug.