1110069 - Replace getBrowser() with gBrowser in browser/

Status: RESOLVED FIXED

(Firefox :: General, defect)

Description

[Dão Gottwald [:dao]]

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

Comment 1

[Ankit Goyal]

Working on it.

Comment 2

[Ankit Goyal]

Attachment: Replaced all getBrowser() with gBrowser() for mentioned files

Comment 3

[Dão Gottwald [:dao]]

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

Comment 4

[Ankit Goyal]

OOPS! Doing again

Comment 5

[Ankit Goyal]

Attachment: fixed

Comment 6

[Ankit Goyal]

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

Comment 7

[Ankit Goyal]

Attachment: done!

Comment 8

[Dão Gottwald [:dao]]

Comment on attachment 8534954 [details] [diff] [review]
done!

Looks good, thanks!

Comment 9

[Dão Gottwald [:dao]]

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?

Comment 10

[Ankit Goyal]

(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

Comment 11

[Ankit Goyal]

(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

Comment 12

[Ankit Goyal]

Shall I do it again?

Comment 13

[Dão Gottwald [:dao]]

Yes, could you please generate a clean diff without editing the result?

Comment 14

[Ankit Goyal]

Ok

Comment 15

[Ankit Goyal]

Attachment: Final patch

Comment 16

[Dão Gottwald [:dao]]

Comment on attachment 8535392 [details] [diff] [review]
Final patch

This one works. Thanks!

Comment 17

[Dão Gottwald [:dao]]

https://hg.mozilla.org/integration/fx-team/rev/85a89a0a4c1c

Comment 18

[Carsten Book [:Tomcat]]

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

Comment 19

[Dão Gottwald [:dao]]

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

Comment 20

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/integration/fx-team/rev/06ac04ca3c31

Comment 21

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

https://hg.mozilla.org/mozilla-central/rev/06ac04ca3c31