671460 - clean up browser_tabview_bug599626.js

Status: RESOLVED FIXED

(Firefox Graveyard :: Panorama, defect)

Description

[Tim Taubert [:ttaubert] (inactive)]

browser_tabview_bug599626.js uses flaky timeouts and is way too complicated, let's refactor it.

Comment 1

[Tim Taubert [:ttaubert] (inactive)]

Attachment: patch v1

Comment 2

[(no longer active)]

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.

Comment 3

[Tim Taubert [:ttaubert] (inactive)]

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

Comment 4

[(no longer active)]

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.  :-)

Comment 5

[Tim Taubert [:ttaubert] (inactive)]

http://hg.mozilla.org/integration/fx-team/rev/f7599a2eb53a

Comment 6

[Tim Taubert [:ttaubert] (inactive)]

Backed out

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

Comment 7

[Tim Taubert [:ttaubert] (inactive)]

http://hg.mozilla.org/integration/fx-team/rev/17eab1719a31

Comment 8

[Tim Taubert [:ttaubert] (inactive)]

http://hg.mozilla.org/mozilla-central/rev/17eab1719a31