Closed Bug 671460 Opened 15 years ago Closed 15 years ago

clean up browser_tabview_bug599626.js

Categories

(Firefox Graveyard :: Panorama, defect)

defect
Not set
normal

Tracking

(Not tracked)

RESOLVED FIXED
Firefox 8

People

(Reporter: ttaubert, Assigned: ttaubert)

Details

(Whiteboard: [cleanup])

Attachments

(1 file)

browser_tabview_bug599626.js uses flaky timeouts and is way too complicated, let's refactor it.
Attached patch patch v1 — — Splinter Review
Attachment #545796 - Flags: review?(ehsan)
Comment on attachment 545796 [details] [diff] [review] patch v1 You're removing the test! Also, can you please explain the refactoring logic? The patch is really hard to read.
Attachment #545796 - Flags: review?(ehsan) → review-
(In reply to comment #2) > You're removing the test! No, I'm only removing the extra .html file because that is now embedded in the test (it's really few html so I figured we don't need the extra file). > Also, can you please explain the refactoring logic? The patch is really > hard to read. So the former approach was to close the tab, wait for the onDOMWillOpenModalDialog event and then check periodically (using setTimeout) if there's a dialog shown. I refactored the patch to not use setTimeout() anymore but use the WindowMediator.onOpenWindow event to wait for the modal dialog to be shown. Besides that I now use some convenience functions like showTabView() and the like. Sorry for not being a bit more communicative about the changes. I'll try to be next time to make it easier for you to review.
Attachment #545796 - Flags: review- → review?(ehsan)
Comment on attachment 545796 [details] [diff] [review] patch v1 Ah, OK. Please in the future use file_bugNNNNNN.html for such files, the name starting with "test_" confused me. :-)
Attachment #545796 - Flags: review?(ehsan) → review+
Whiteboard: [cleanup] → [cleanup][fixed-in-fx-team]
Status: ASSIGNED → RESOLVED
Closed: 15 years ago
Resolution: --- → FIXED
Whiteboard: [cleanup][fixed-in-fx-team] → [cleanup]
Target Milestone: --- → Firefox 8
Product: Firefox → Firefox Graveyard
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: