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

Status: RESOLVED FIXED

(Testing Graveyard :: Mozmill, defect)

Description

[Henrik Skupin [:whimboo][⌚️UTC+1]]

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.

Comment 1

[Henrik Skupin [:whimboo][⌚️UTC+1]]

This is a regression from bug 696494 which we would have to fix.

Comment 2

[Henrik Skupin [:whimboo][⌚️UTC+1]]

Attachment: Patch v1

Pointer to Github pull-request

Comment 3

[Henrik Skupin [:whimboo][⌚️UTC+1]]

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.

Comment 4

[Andrew Halberstadt [:ahal]]

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.

Comment 5

[Henrik Skupin [:whimboo][⌚️UTC+1]]

(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.

Comment 6

[Henrik Skupin [:whimboo][⌚️UTC+1]]

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