Implement Firefox TabBrowser API: getIcon()

RESOLVED FIXED in seamonkey2.1a2

Status

SeaMonkey
UI Design
RESOLVED FIXED
7 years ago
7 years ago

People

(Reporter: sgautherie, Assigned: sgautherie)

Tracking

(Blocks: 1 bug)

Trunk
seamonkey2.1a2
Dependency tree / graph
Bug Flags:
in-testsuite -

Firefox Tracking Flags

(Not tracked)

Details

(URL)

Attachments

(1 attachment)

(Assignee)

Description

7 years ago
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
(Assignee)

Comment 1

7 years ago
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.
Attachment #438373 - Flags: superreview?(neil)
Attachment #438373 - Flags: review?(neil)

Updated

7 years ago
Blocks: 554908

Comment 2

7 years ago
review ping

Updated

7 years ago
Attachment #438373 - Flags: review?(bugspam.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.
Attachment #438373 - Flags: superreview?(neil)
Attachment #438373 - Flags: review?(neil)
Attachment #438373 - Flags: review?(bugspam.Callek)
Attachment #438373 - Flags: review+
(Assignee)

Comment 4

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


http://hg.mozilla.org/comm-central/rev/650b6d05de95
Attachment #438373 - Attachment description: (Av1) Just copy it from Firefox → (Av1) Just copy it from Firefox [Checkin: Comment 4]
(Assignee)

Updated

7 years ago
Status: ASSIGNED → RESOLVED
Last Resolved: 7 years ago
Depends on: 525708
Flags: in-testsuite-
Resolution: --- → FIXED
Target Milestone: --- → seamonkey2.1a2

Comment 5

7 years ago
+          <![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.
(Assignee)

Comment 6

7 years ago
(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

7 years ago
> 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

7 years ago
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).
(Assignee)

Comment 9

7 years ago
(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()?
You need to log in before you can comment on or make changes to this bug.