properly test for TabView being defined

RESOLVED FIXED in Firefox 6

Status

()

Firefox
Tabbed Browser
P3
normal
RESOLVED FIXED
7 years ago
6 years ago

People

(Reporter: iangilman, Assigned: raymondlee)

Tracking

Trunk
Firefox 6
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 2 obsolete attachments)

(Reporter)

Description

7 years ago
[1:33pm] Mardak: you'll get a ReferenceError: TabView is not defined if you dont do window.TabView

This may fix bug 586818, but even if not, it's worth doing.
(Reporter)

Comment 1

7 years ago
Created attachment 465872 [details] [diff] [review]
Patch v1
Attachment #465872 - Flags: review?(dolske)
Comment on attachment 465872 [details] [diff] [review]
Patch v1

I suppose this could alternatively use |typeof TabView != "undefined"|, but whatever.
Attachment #465872 - Flags: review?(dolske)
Attachment #465872 - Flags: review+
Attachment #465872 - Flags: approval2.0+

Comment 3

7 years ago
http://hg.mozilla.org/mozilla-central/rev/5deb7e7751d8
Check the global object for existence to avoid ReferenceError.
Status: NEW → RESOLVED
Last Resolved: 7 years ago
Resolution: --- → FIXED
Whiteboard: b4
Target Milestone: --- → Firefox 4.0b4
Version: unspecified → Trunk

Comment 4

7 years ago
backed out http://hg.mozilla.org/mozilla-central/rev/c14d91f2d939
Status: RESOLVED → REOPENED
Resolution: FIXED → ---

Updated

7 years ago
Component: TabCandy → Tabbed Browser
QA Contact: tabcandy → tabbed.browser
Comment on attachment 465872 [details] [diff] [review]
Patch v1

>diff -r e49b735ae874 browser/base/content/tabbrowser.xml
>--- a/browser/base/content/tabbrowser.xml	Thu Aug 12 18:18:25 2010 -0700
>+++ b/browser/base/content/tabbrowser.xml	Fri Aug 13 15:43:27 2010 -0700
>@@ -723,7 +723,7 @@
>       <method name="updateTitlebar">
>         <body>
>           <![CDATA[
>-            if (TabView && TabView.isVisible()) {
>+            if (window.TabView && TabView.isVisible()) {

Is this going to spawn a "reference to undefined property window.TabView" (strict?) warning? ("TabView" in window && Taview...) might be better.

Comment 6

7 years ago
http://hg.mozilla.org/mozilla-central/rev/0865f00c7873
Status: REOPENED → RESOLVED
Last Resolved: 7 years ago7 years ago
Resolution: --- → FIXED
reopening, see comment 5
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Summary: test for window.TabView rather than just TabView → properly test for TabView being defined
No longer blocks: 586788
Target Milestone: Firefox 4.0b4 → Firefox 4.0
(Reporter)

Updated

7 years ago
Priority: -- → P3
(Reporter)

Updated

7 years ago
Blocks: 626527
Whiteboard: [approved-patches-landed]
(Reporter)

Comment 8

7 years ago
Raymond, this has been languishing on my plate. Can you take care of it?
Assignee: ian → raymond
(Assignee)

Comment 9

6 years ago
Created attachment 525928 [details] [diff] [review]
v2
Attachment #465872 - Attachment is obsolete: true
Attachment #525928 - Flags: feedback?(tim.taubert)
Comment on attachment 525928 [details] [diff] [review]
v2

Looks good :)
Attachment #525928 - Flags: feedback?(tim.taubert) → feedback+
(Assignee)

Comment 11

6 years ago
Comment on attachment 525928 [details] [diff] [review]
v2

Passed Try
http://tbpl.mozilla.org/?tree=MozillaTry&rev=51afe70f85d8
Attachment #525928 - Flags: review?(ian)
(Reporter)

Comment 12

6 years ago
Comment on attachment 525928 [details] [diff] [review]
v2

Thanks!
Attachment #525928 - Flags: review?(ian) → review+
(Assignee)

Comment 13

6 years ago
Created attachment 526379 [details] [diff] [review]
Patch for checkin
Attachment #525928 - Attachment is obsolete: true
(Assignee)

Updated

6 years ago
Keywords: checkin-needed
http://hg.mozilla.org/mozilla-central/rev/b425640ea556
Status: REOPENED → RESOLVED
Last Resolved: 7 years ago6 years ago
Keywords: checkin-needed
Resolution: --- → FIXED
Whiteboard: [approved-patches-landed]
Target Milestone: Firefox 4.0 → Firefox 6
You need to log in before you can comment on or make changes to this bug.