Replace getBrowser() with gBrowser in browser/

RESOLVED FIXED in Firefox 37

Status

()

Firefox
General
RESOLVED FIXED
4 years ago
4 years ago

People

(Reporter: dao, Assigned: Ankit Goyal, Mentored)

Tracking

Trunk
Firefox 37
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [good first bug][lang=js])

Attachments

(1 attachment, 3 obsolete attachments)

(Reporter)

Description

4 years ago
Bug 448572 deprecated getBrowser() a long time ago. We should be using gBrowser instead.

The instances that should be replaced can be found here:

http://mxr.mozilla.org/mozilla-central/search?string=getBrowser%28%29&find=%2Fbrowser%2F

From that list, you can ignore browser/base/content/test/general/browser_scope.js and anything under browser/metro/.
(Assignee)

Comment 1

4 years ago
Working on it.
(Assignee)

Comment 2

4 years ago
Created attachment 8534946 [details] [diff] [review]
Replaced all getBrowser() with gBrowser() for mentioned files
(Reporter)

Comment 3

4 years ago
Comment on attachment 8534946 [details] [diff] [review]
Replaced all getBrowser() with gBrowser() for mentioned files

>-    var uri = getBrowser().currentURI;
>+    var uri = gBrowser().currentURI;

gBrowser is not a function but a DOM node (the same DOM node returned by getBrowser()). So you need to replace "getBrowser()" with "gBrowser", no parentheses.

>--- a/browser/base/content/browser.js
>+++ b/browser/base/content/browser.js
>@@ -7381,17 +7381,16 @@ function getNotificationBox(aWindow) {
> function getTabModalPromptBox(aWindow) {
>   var foundBrowser = gBrowser.getBrowserForDocument(aWindow.document);
>   if (foundBrowser)
>     return gBrowser.getTabModalPromptBox(foundBrowser);
>   return null;
> };
> 
> /* DEPRECATED */
>-function getBrowser() gBrowser;

We need to keep this for add-ons that might be calling getBrowser().
(Assignee)

Comment 4

4 years ago
OOPS! Doing again
(Assignee)

Comment 5

4 years ago
Created attachment 8534953 [details] [diff] [review]
fixed
(Assignee)

Comment 6

4 years ago
(In reply to ankit.goyal90 from comment #5)
> Created attachment 8534953 [details] [diff] [review]
> fixed

OOPS again. Looks you have to apply both patches. I create a single one.
(Assignee)

Comment 7

4 years ago
Created attachment 8534954 [details] [diff] [review]
done!
(Reporter)

Comment 8

4 years ago
Comment on attachment 8534954 [details] [diff] [review]
done!

Looks good, thanks!
Attachment #8534954 - Flags: review+
(Reporter)

Updated

4 years ago
Attachment #8534953 - Attachment is obsolete: true
(Reporter)

Updated

4 years ago
Attachment #8534946 - Attachment is obsolete: true
(Reporter)

Comment 9

4 years ago
Hm, the patch fails to apply in browser/base/content/browser-safebrowsing.js. Not sure what went wrong there. Have you manually edited the patch?
(Reporter)

Updated

4 years ago
Assignee: nobody → ankit.goyal90
(Assignee)

Comment 10

4 years ago
(In reply to Dão Gottwald [:dao] from comment #9)
> Hm, the patch fails to apply in
> browser/base/content/browser-safebrowsing.js. Not sure what went wrong
> there. Have you manually edited the patch?

Yes
(Assignee)

Comment 11

4 years ago
(In reply to Dão Gottwald [:dao] from comment #9)
> Hm, the patch fails to apply in
> browser/base/content/browser-safebrowsing.js. Not sure what went wrong
> there. Have you manually edited the patch?

Yes
(Assignee)

Comment 12

4 years ago
Shall I do it again?
(Reporter)

Comment 13

4 years ago
Yes, could you please generate a clean diff without editing the result?
(Assignee)

Comment 14

4 years ago
Ok
(Assignee)

Comment 15

4 years ago
Created attachment 8535392 [details] [diff] [review]
Final patch
(Reporter)

Comment 16

4 years ago
Comment on attachment 8535392 [details] [diff] [review]
Final patch

This one works. Thanks!
Attachment #8535392 - Flags: review+
(Reporter)

Updated

4 years ago
Attachment #8534954 - Attachment is obsolete: true
sorry had to back this out in https://treeherder.mozilla.org/#/jobs?repo=fx-team&revision=2a11e49c5a83 since one of this 2 changes caused test failures like https://treeherder.mozilla.org/logviewer.html#?job_id=1424338&repo=fx-team
Flags: needinfo?(ankit.goyal90)
(Reporter)

Comment 19

4 years ago
(In reply to Carsten Book [:Tomcat] from comment #18)
> sorry had to back this out in
> https://treeherder.mozilla.org/#/jobs?repo=fx-team&revision=2a11e49c5a83
> since one of this 2 changes caused test failures like
> https://treeherder.mozilla.org/logviewer.html#?job_id=1424338&repo=fx-team

This had nothing to do with this bug.
Flags: needinfo?(ankit.goyal90)
Keywords: checkin-needed
https://hg.mozilla.org/integration/fx-team/rev/06ac04ca3c31
Keywords: checkin-needed
Whiteboard: [good first bug][lang=js] → [good first bug][lang=js][fixed-in-fx-team]
https://hg.mozilla.org/mozilla-central/rev/06ac04ca3c31
Status: NEW → RESOLVED
Last Resolved: 4 years ago
Resolution: --- → FIXED
Whiteboard: [good first bug][lang=js][fixed-in-fx-team] → [good first bug][lang=js]
Target Milestone: --- → Firefox 37
You need to log in before you can comment on or make changes to this bug.