Tab is Undefined error in Error Console - Win7 Previews

RESOLVED FIXED in Firefox 3.7a1

Status

()

Firefox
General
RESOLVED FIXED
9 years ago
8 years ago

People

(Reporter: Jim Jeffery not reading bug-mail 1/2/11, Assigned: dao)

Tracking

({verified1.9.2})

3.6 Branch
Firefox 3.7a1
x86
Windows 7
verified1.9.2
Points:
---
Bug Flags:
in-testsuite +

Firefox Tracking Flags

(status1.9.2 beta1-fixed)

Details

Attachments

(2 attachments)

This error is appearing in the Console2 Error Console:

Error: tab is undefined
Source file: file:///J:/Program%20Files%20(x86)/Minefield/modules/WindowsPreviewPerTab.jsm
Line: 514

Not sure yet of what's triggering the error.  Seems it might be related to sites doing a periodic refresh of page, i.e.: msnbc perhaps.  

Using an hourly build:
Mozilla/5.0 (Windows; U; Windows NT 6.1; en-US; rv:1.9.3a1pre) Gecko/20091007 Minefield/3.7a1pre (.NET CLR 3.5.30729) ID:20091007231829

cset: http://hg.mozilla.org/mozilla-central/rev/d8c914dbd3f8
As I clicked 'commit' while filing this bug I got the error to appear.
(Assignee)

Comment 2

9 years ago
Rob: 'for each' shouldn't be used like this. There's no good reason to use it on arrays, and it's plain wrong on node lists. See also the warning here: <https://developer.mozilla.org/en/Core_JavaScript_1.5_Reference/Statements/for_each...in#Examples>
Blocks: 474056
Component: Widget: Win32 → General
Product: Core → Firefox
QA Contact: win32 → general
Version: Trunk → 3.6 Branch
(Assignee)

Comment 3

9 years ago
fixing...
Assignee: nobody → dao
(Assignee)

Comment 4

9 years ago
Created attachment 405251 [details] [diff] [review]
patch
Attachment #405251 - Flags: review?(tellrob)
(Assignee)

Updated

9 years ago
OS: Windows NT → Windows 7
Comment on attachment 405251 [details] [diff] [review]
patch

Looks good. Wish I'd known that sooner!
Attachment #405251 - Flags: review?(tellrob) → review+
Status: NEW → ASSIGNED
(Assignee)

Comment 6

9 years ago
http://hg.mozilla.org/mozilla-central/rev/f4c973091e28
Status: ASSIGNED → RESOLVED
Last Resolved: 9 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 3.7a1
(Assignee)

Updated

9 years ago
Attachment #405251 - Flags: approval1.9.2?
Comment on attachment 405251 [details] [diff] [review]
patch

>@@ -367,18 +367,18 @@ XPCOMUtils.defineLazyGetter(PreviewContr
>-  for each (let evtName in this.events)
>-    this.tabbrowser.tabContainer.addEventListener(evtName, this, false);
>+  for (let i = 0; i < this.events.length; i++)
>+    this.tabbrowser.tabContainer.addEventListener(this.events[i], this, false);
Why not use .forEach? Is that not an actual array []?

Array.forEach(this.events, function(event) {
  this.tabbrowser.tabContainer.addEventListener(event, this, false);
}, this);
Oh, it is a plain array, so this.events.forEach should work
(Assignee)

Comment 9

9 years ago
Yes, that it would have worked as well. Since it's a one-liner (allowing me to drop the brackets) and this.events[i] appears only once, I don't think forEach would have improved the code significantly. I also wonder if forEach is actually still faster.
Ok, I just downloaded the latest hourly that should contain this patch, and I still see the error..

cset: http://hg.mozilla.org/mozilla-central/rev/542fa9413bd0

Mozilla/5.0 (Windows; U; Windows NT 6.1; en-US; rv:1.9.3a1pre) Gecko/20091008 Minefield/3.7a1pre Firefox/3.7 (.NET CLR 3.5.30729) ID:20091008114812

Error: this.previews[index] is undefined
Source file: file:///J:/Program%20Files%20(x86)/Minefield/modules/WindowsPreviewPerTab.jsm
Line: 514
I can still trigger the error by simply opening a bug from history.

Or middle-clicking a link from the location bar.
(Assignee)

Comment 12

9 years ago
Landed this to get more useful information:
http://hg.mozilla.org/mozilla-central/rev/7e78bc8c98ea

Please report back with new console output.
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
(In reply to comment #10)
> Ok, I just downloaded the latest hourly that should contain this patch, and I
> still see the error..
> 
> cset: http://hg.mozilla.org/mozilla-central/rev/542fa9413bd0
> 
> Mozilla/5.0 (Windows; U; Windows NT 6.1; en-US; rv:1.9.3a1pre) Gecko/20091008
> Minefield/3.7a1pre Firefox/3.7 (.NET CLR 3.5.30729) ID:20091008114812
> 
> Error: this.previews[index] is undefined
> Source file:
> file:///J:/Program%20Files%20(x86)/Minefield/modules/WindowsPreviewPerTab.jsm
> Line: 514

Did you clean the error counsel first before installing the new build?
@ cuz84d
Yes, I always clear the console2 before starting to test for bugs like this.

I just got the build with debug patch and here is the output:
1.  Middle-clicked a link from the location-bar to open in a tab

Error: 13,14,14,13
Source file: file:///J:/Program%20Files%20(x86)/Minefield/modules/WindowsPreviewPerTab.jsm
Line: 517
The error seems to change, here is another from mid-click on link from location-bar drop list:

Error: 11,12,12,11
Source file: file:///J:/Program%20Files%20(x86)/Minefield/modules/WindowsPreviewPerTab.jsm
Line: 517
(Assignee)

Comment 16

9 years ago
This might be a tabbrowser bug. I've pushed something to the tryserver for testing.
(Assignee)

Comment 19

9 years ago
Created attachment 405455 [details] [diff] [review]
tabbrowser patch

So the problem is that the tabs progress listener gets notifications about the new browser before the TabOpen event is dispatched.
(In reply to comment #19)
> Created an attachment (id=405455) [details]
> tabbrowser patch
> 
> So the problem is that the tabs progress listener gets notifications about the
> new browser before the TabOpen event is dispatched.

Not to butt in, but you didn't mark this for review ?
(Assignee)

Comment 21

9 years ago
Comment on attachment 405455 [details] [diff] [review]
tabbrowser patch

passed all unit tests on the tryserver
Attachment #405455 - Flags: review?(gavin.sharp)
Attachment #405455 - Flags: review?(gavin.sharp) → review+
(Assignee)

Comment 22

9 years ago
http://hg.mozilla.org/mozilla-central/rev/b2057b7554e6
Status: REOPENED → RESOLVED
Last Resolved: 9 years ago9 years ago
Flags: in-testsuite+
Resolution: --- → FIXED
(Assignee)

Updated

9 years ago
Attachment #405455 - Flags: approval1.9.2?
Attachment #405251 - Flags: approval1.9.2? → approval1.9.2+
Attachment #405455 - Flags: approval1.9.2? → approval1.9.2+
Comment on attachment 405455 [details] [diff] [review]
tabbrowser patch

a192=beltzner
(Assignee)

Updated

9 years ago
Keywords: checkin-needed
why is this still checkin-needed? what's left to do?
(Assignee)

Updated

9 years ago
Keywords: checkin-needed
I had to take these on the release branch so bug 522416 would apply cleanly and I didn't have to figure out a special patch just for release branch for beta 1.  If this was wrong of me to do, I'm sorry and we can back it out.

http://hg.mozilla.org/releases/mozilla-1.9.2/rev/27d9d4fb4064
http://hg.mozilla.org/releases/mozilla-1.9.2/rev/a5340b714f5f
status1.9.2: final-fixed → beta1-fixed
Verified fixed on the 1.9.2 branch using  Mozilla/5.0 (Windows; U; Windows NT 6.1; en-US; rv:1.9.2b6pre) Gecko/20100105 Namoroka/3.6b6pre. I verified by opening links from the location bar with middle clicks and opening history entries.
Keywords: verified1.9.2
You need to log in before you can comment on or make changes to this bug.