Last Comment Bug 558673 - Implement Firefox TabBrowser API: getIcon()
: Implement Firefox TabBrowser API: getIcon()
Status: RESOLVED FIXED
:
Product: SeaMonkey
Classification: Client Software
Component: UI Design (show other bugs)
: Trunk
: All All
: -- normal (vote)
: seamonkey2.1a2
Assigned To: Serge Gautherie (:sgautherie)
:
Mentors:
http://mxr.mozilla.org/comm-central/s...
Depends on: 525708
Blocks: SMtabAPI 554908 556684
  Show dependency treegraph
 
Reported: 2010-04-11 11:06 PDT by Serge Gautherie (:sgautherie)
Modified: 2010-09-19 01:42 PDT (History)
4 users (show)
bugzillamozillaorg_serge_20140323: in‑testsuite-
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---


Attachments
(Av1) Just copy it from Firefox [Checkin: Comment 4] (983 bytes, patch)
2010-04-11 11:13 PDT, Serge Gautherie (:sgautherie)
bugspam.Callek: review+
Details | Diff | Splinter Review

Description Serge Gautherie (:sgautherie) 2010-04-11 11:06:44 PDT
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) 2010-04-11 11:13:13 PDT
Created attachment 438373 [details] [diff] [review]
(Av1) Just copy it from Firefox
[Checkin: Comment 4]

This doesn't (fully) fix, but it helps, bug 556684.
Comment 2 Misak Khachatryan 2010-06-18 05:32:17 PDT
review ping
Comment 3 Justin Wood (:Callek) 2010-06-18 19:06:53 PDT
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) 2010-06-19 06:23:13 PDT
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 2010-06-19 07:42:12 PDT
+          <![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) 2010-06-19 08:29:33 PDT
(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 2010-06-19 09:20:42 PDT
> 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 neil@parkwaycc.co.uk 2010-06-22 03:21:41 PDT
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) 2010-09-19 01:42:57 PDT
(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()?

Note You need to log in before you can comment on or make changes to this bug.