properly test for TabView being defined

RESOLVED FIXED in Firefox 6

Status

()

defect
P3
normal
RESOLVED FIXED
9 years ago
9 years ago

People

(Reporter: iangilman, Assigned: raymondlee)

Tracking

Trunk
Firefox 6
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 2 obsolete attachments)

[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.
Posted patch Patch v1 (obsolete) — Splinter Review
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+
http://hg.mozilla.org/mozilla-central/rev/5deb7e7751d8
Check the global object for existence to avoid ReferenceError.
Status: NEW → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
Whiteboard: b4
Target Milestone: --- → Firefox 4.0b4
Version: unspecified → Trunk
backed out http://hg.mozilla.org/mozilla-central/rev/c14d91f2d939
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
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.
http://hg.mozilla.org/mozilla-central/rev/0865f00c7873
Status: REOPENED → RESOLVED
Closed: 9 years ago9 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
Priority: -- → P3
Blocks: 626527
Whiteboard: [approved-patches-landed]
Raymond, this has been languishing on my plate. Can you take care of it?
Assignee: ian → raymond
Posted patch v2 (obsolete) — Splinter Review
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+
Comment on attachment 525928 [details] [diff] [review]
v2

Thanks!
Attachment #525928 - Flags: review?(ian) → review+
Attachment #525928 - Attachment is obsolete: true
Keywords: checkin-needed
http://hg.mozilla.org/mozilla-central/rev/b425640ea556
Status: REOPENED → RESOLVED
Closed: 9 years ago9 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.