Closed Bug 587187 Opened 14 years ago Closed 14 years ago

properly test for TabView being defined

Categories

(Firefox :: Tabbed Browser, defect, P3)

Product:
Firefox
Component:
Tabbed Browser
Type:
defect

Tracking

()

Status:
RESOLVED FIXED
Milestone:
Firefox 6

People

(Reporter: iangilman, Assigned: raymondlee)

References

Details

Attachments

(1 file, 2 obsolete files)

Patch for checkin
(deleted), patch
Details | Diff | Splinter Review
[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.
Attached patch Patch v1 (obsolete) (deleted) — — 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: 14 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: 14 years ago → 14 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
Attached patch v2 (obsolete) (deleted) — — 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+
Attached patch Patch for checkin (deleted) — — Splinter Review
Attachment #525928 - Attachment is obsolete: true
Keywords: checkin-needed
http://hg.mozilla.org/mozilla-central/rev/b425640ea556
Status: REOPENED → RESOLVED
Closed: 14 years ago → 14 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.

Attachment

General

Creator:
Created:
Updated:
Size: