Closed Bug 587187 Opened 14 years ago Closed 13 years ago

properly test for TabView being defined

Tracking

()

Status:

RESOLVED FIXED

Milestone:

Firefox 6

People

(Reporter: iangilman, Assigned: raymondlee)

References

Details

Attachments

(1 file, 2 obsolete files)

Patch v1 14 years ago Ian Gilman [:iangilman] 590 bytes, patch	Dolske : review+ Dolske : approval2.0+	Details \| Diff \| Splinter Review
v2 13 years ago Raymond Lee [:raymondlee] 1.57 KB, patch	iangilman : review+ ttaubert : feedback+	Details \| Diff \| Splinter Review
Patch for checkin 13 years ago Raymond Lee [:raymondlee] 1.55 KB, patch		Details \| Diff \| Splinter Review

Ian Gilman [:iangilman]

Reporter

Description

•

14 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.

Ian Gilman [:iangilman]

Reporter

Comment 1

•

14 years ago

Attached patch Patch v1 (obsolete) — Details — Splinter Review

Attachment #465872 - Flags: review?(dolske)

Justin Dolske [:Dolske]

Comment 2

•

14 years ago

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+

Ed Lee :Mardak

Comment 3

•

14 years ago

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

Ed Lee :Mardak

Comment 4

•

14 years ago

backed out http://hg.mozilla.org/mozilla-central/rev/c14d91f2d939

Status: RESOLVED → REOPENED

Resolution: FIXED → ---

Dão Gottwald [:dao]

Updated

•

14 years ago

Component: TabCandy → Tabbed Browser

QA Contact: tabcandy → tabbed.browser

Dão Gottwald [:dao]

Comment 5

•

14 years ago

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.

Ed Lee :Mardak

Comment 6

•

14 years ago

http://hg.mozilla.org/mozilla-central/rev/0865f00c7873

Status: REOPENED → RESOLVED

Closed: 14 years ago → 14 years ago

Resolution: --- → FIXED

Dão Gottwald [:dao]

Comment 7

•

14 years ago

reopening, see comment 5

Status: RESOLVED → REOPENED

Resolution: FIXED → ---

Summary: test for window.TabView rather than just TabView → properly test for TabView being defined

Michael Yoshitaka Erlewine [:mitcho]

Updated

•

14 years ago

No longer blocks: 586788

Michael Yoshitaka Erlewine [:mitcho]

Updated

•

14 years ago

Target Milestone: Firefox 4.0b4 → Firefox 4.0

Ian Gilman [:iangilman]

Reporter

Updated

•

14 years ago

Priority: -- → P3

Ian Gilman [:iangilman]

Reporter

Updated

•

14 years ago

Blocks: 626527

Joe Drew (not getting mail)

Updated

•

13 years ago

Whiteboard: [approved-patches-landed]

Ian Gilman [:iangilman]

Reporter

Comment 8

•

13 years ago

Raymond, this has been languishing on my plate. Can you take care of it?

Assignee: ian → raymond

Raymond Lee [:raymondlee]

Assignee

Comment 9

•

13 years ago

Attached patch v2 (obsolete) — Details — Splinter Review

Attachment #465872 - Attachment is obsolete: true

Attachment #525928 - Flags: feedback?(tim.taubert)

Tim Taubert [:ttaubert] (inactive)

Comment 10

•

13 years ago

Comment on attachment 525928 [details] [diff] [review]
v2

Looks good :)

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

Raymond Lee [:raymondlee]

Assignee

Comment 11

•

13 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)

Ian Gilman [:iangilman]

Reporter

Comment 12

•

13 years ago

Comment on attachment 525928 [details] [diff] [review]
v2

Thanks!

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

Raymond Lee [:raymondlee]

Assignee

Comment 13

•

13 years ago

Attached patch Patch for checkin — Details — Splinter Review

Attachment #525928 - Attachment is obsolete: true

Raymond Lee [:raymondlee]

Assignee

Updated

•

13 years ago

Keywords: checkin-needed

:Ms2ger (he/him; ⌚ UTC+1/+2)

Comment 14

•

13 years ago

http://hg.mozilla.org/mozilla-central/rev/b425640ea556

Status: REOPENED → RESOLVED

Closed: 14 years ago → 13 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.