Note: There are a few cases of duplicates in user autocompletion which are being worked on.

clean up browser_tabview_bug599626.js

RESOLVED FIXED in Firefox 8

Status

Firefox Graveyard
Panorama
RESOLVED FIXED
6 years ago
a year ago

People

(Reporter: ttaubert, Assigned: ttaubert)

Tracking

Trunk
Firefox 8

Details

(Whiteboard: [cleanup])

Attachments

(1 attachment)

(Assignee)

Description

6 years ago
browser_tabview_bug599626.js uses flaky timeouts and is way too complicated, let's refactor it.
(Assignee)

Comment 1

6 years ago
Created attachment 545796 [details] [diff] [review]
patch v1
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-
(Assignee)

Comment 3

6 years ago
(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.
(Assignee)

Updated

6 years ago
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+
(Assignee)

Comment 5

6 years ago
http://hg.mozilla.org/integration/fx-team/rev/f7599a2eb53a
Whiteboard: [cleanup] → [cleanup][fixed-in-fx-team]
(Assignee)

Comment 6

6 years ago
Backed out

http://tbpl.mozilla.org/?tree=Fx-Team&rev=f7599a2eb53a
(Assignee)

Comment 7

6 years ago
http://hg.mozilla.org/integration/fx-team/rev/17eab1719a31
(Assignee)

Comment 8

6 years ago
http://hg.mozilla.org/mozilla-central/rev/17eab1719a31
Status: ASSIGNED → RESOLVED
Last Resolved: 6 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.