Last Comment Bug 633096 - Rewrite afterAllTabItemsUpdated in head.js using a tabItem observer
: Rewrite afterAllTabItemsUpdated in head.js using a tabItem observer
Status: RESOLVED FIXED
:
Product: Firefox Graveyard
Classification: Graveyard
Component: Panorama (show other bugs)
: Trunk
: All All
: P4 normal
: ---
Assigned To: Tim Taubert [:ttaubert]
:
Mentors:
Depends on: 602432 659594
Blocks:
  Show dependency treegraph
 
Reported: 2011-02-09 22:55 PST by Sean Dunn
Modified: 2016-04-12 14:00 PDT (History)
5 users (show)
See Also:
QA Whiteboard:
Iteration: ---
Points: ---


Attachments
patch v1 (4.15 KB, patch)
2011-04-09 22:42 PDT, Tim Taubert [:ttaubert]
ian: review+
raymond: feedback+
Details | Diff | Review
patch v2 (9.44 KB, patch)
2011-04-24 07:16 PDT, Tim Taubert [:ttaubert]
ian: review+
Details | Diff | Review

Description Sean Dunn 2011-02-09 22:55:58 PST
Right now in head.js's afterAllTabItemsUpdated we call update on each of them, essentially bypassing the TabPriorityQueue. Adding the _updateTestFunc hook to  check that they're all updated and then calling update() on them is much more graceful. Rewrite afterAllTabItemsUpdated in terms of this kind of  tabPriorityQueue-obeying logic, and making sure all the tests still pass.
Comment 1 Kevin Hanes 2011-03-31 10:51:49 PDT
bugspam
Comment 2 Tim Taubert [:ttaubert] 2011-04-09 22:42:12 PDT
Created attachment 524910 [details] [diff] [review]
patch v1
Comment 4 Raymond Lee [:raymondlee] 2011-04-10 12:43:19 PDT
Comment on attachment 524910 [details] [diff] [review]
patch v1

Looks good to me.
Comment 5 Ian Gilman [:iangilman] 2011-04-12 10:14:42 PDT
Comment on attachment 524910 [details] [diff] [review]
patch v1

Looks fine, but can you explain why those two tests had to be changed?
Comment 6 Tim Taubert [:ttaubert] 2011-04-24 07:16:23 PDT
Created attachment 528001 [details] [diff] [review]
patch v2

(In reply to comment #5)
> Looks fine, but can you explain why those two tests had to be changed?

browser_tabview_expander.js:

The problem is that newly created tabs with the url "about:blank#something" have a readyState == "uninitialized". So TabItem.isComplete() will never return true and the update will never occur. That is why I changed the url to "about:home". Alternatively we could also remove the hash.

browser_tabview_bug594958.js:

In bug 625561 (http://hg.mozilla.org/mozilla-central/rev/20ffcaad9700) these values got changed and I can't really remember why. So what I did was to re-think the whole test and to re-check the given values so that they're now correct and the thumbnails are rendered as expected. I think this will also help us the continue with bug 641188.
Comment 7 Tim Taubert [:ttaubert] 2011-04-24 07:46:47 PDT
(In reply to comment #6)
> I think this will also help us the continue with bug 641188.

It does. I'll post in bug 641188.
Comment 8 Ian Gilman [:iangilman] 2011-04-26 10:21:36 PDT
Comment on attachment 528001 [details] [diff] [review]
patch v2

Review of attachment 528001 [details] [diff] [review]:

Cool, thanks
Comment 9 Tim Taubert [:ttaubert] 2011-05-27 02:22:56 PDT
bugspam
Comment 10 Tim Taubert [:ttaubert] 2011-05-27 02:27:47 PDT
bugspam
Comment 11 Tim Taubert [:ttaubert] 2011-07-24 18:38:41 PDT
bugspam

(Fx7 was branched, removing open bugs from Fx7 meta bug, we won't create new meta bugs for upcoming Fx versions)
Comment 12 Raymond Lee [:raymondlee] 2011-09-27 21:14:33 PDT
Is there any concerns why this patch is not checked-in?
Comment 13 Tim Taubert [:ttaubert] 2011-09-28 04:59:46 PDT
Yeah, alas, this patch does not pass try. I tried to find the error source but wasn't very successful so far and didn't have much time to try it further (because that's such a minor issue).
Comment 14 Tim Taubert [:ttaubert] 2012-04-26 12:14:43 PDT
Fixed by bug 659594.

Note You need to log in before you can comment on or make changes to this bug.