558673 - Implement Firefox TabBrowser API: getIcon()

Status: RESOLVED FIXED

(SeaMonkey :: UI Design, defect)

Description

[Serge Gautherie (:sgautherie)]

http://mxr.mozilla.org/mozilla-central/search?string=getIcon&case=1
{
/browser/base/content/tabbrowser.xml
    * line 658 -- <method name="getIcon">
}

Currently needed by bug 556684
/docshell/test/browser/browser_bug550565.js

Comment 1

[Serge Gautherie (:sgautherie)]

Attachment: (Av1) Just copy it from Firefox [Checkin: Comment 4]

This doesn't (fully) fix, but it helps, bug 556684.

Comment 2

[Misak Khachatryan]

review ping

Comment 3

[Justin Wood (:Callek)]

Comment on attachment 438373 [details] [diff] [review]
(Av1) Just copy it from Firefox
[Checkin: Comment 4]

Stealing review, going for the "no sr needed" route.

Comment 4

[Serge Gautherie (:sgautherie)]

Comment on attachment 438373 [details] [diff] [review]
(Av1) Just copy it from Firefox
[Checkin: Comment 4]


http://hg.mozilla.org/comm-central/rev/650b6d05de95

Comment 5

[Philip Chee]

+          <![CDATA[
+            let browser = aTab ? this.getBrowserForTab(aTab) : this.selectedBrowser;
Bah. Use |var| for top level variables. Spidermonkey internally converts top level lets to vars anyway.

Comment 6

[Serge Gautherie (:sgautherie)]

(In reply to comment #5)
> +          <![CDATA[
> +            let browser = aTab ? this.getBrowserForTab(aTab) :
> this.selectedBrowser;
> Bah. Use |var| for top level variables. Spidermonkey internally converts top
> level lets to vars anyway.

Fwiw, is method == top level ?

Comment 7

[Philip Chee]

> Fwiw, is method == top level ?

let|var browser is at the top of the scope of the "getIcon" method so, yes. The sytle guide for suite/ is to use lets only for sub-scope variables e.g. inside for/if/while/try blocks and only if there is a chance of ambiguity e.g. if you have several |for (var i = 0; i < foo: i++)| then you should use |for (let i = 0; i < foo: i++)|

Comment 8

[[e-mail]]

Comment on attachment 438373 [details] [diff] [review]
(Av1) Just copy it from Firefox
[Checkin: Comment 4]

>+            let browser = aTab ? this.getBrowserForTab(aTab) : this.selectedBrowser;
This could also have used .linkedBrowser (getBrowserForTab also only exists for Firefox compatibility).

Comment 9

[Serge Gautherie (:sgautherie)]

(In reply to comment #8)
> This could also have used .linkedBrowser (getBrowserForTab also only exists for
> Firefox compatibility).

Ftr,
*Done in bug 554908.
*http://mxr.mozilla.org/comm-central/search?string=getBrowserForTab&case=on&find=%2Fsuite%2F
 "Found 17 matching lines in 9 files"
 Should we update "all" these too? And add a comment on getBrowserForTab()?