Closed
Bug 671460
Opened 15 years ago
Closed 15 years ago
clean up browser_tabview_bug599626.js
Categories
(Firefox Graveyard :: Panorama, defect)
Firefox Graveyard
Panorama
Tracking
(Not tracked)
RESOLVED
FIXED
Firefox 8
People
(Reporter: ttaubert, Assigned: ttaubert)
Details
(Whiteboard: [cleanup])
Attachments
(1 file)
|
8.62 KB,
patch
|
ehsan.akhgari
:
review+
|
Details | Diff | Splinter Review |
browser_tabview_bug599626.js uses flaky timeouts and is way too complicated, let's refactor it.
| Assignee | ||
Comment 1•15 years ago
|
||
Attachment #545796 -
Flags: review?(ehsan)
Comment 2•15 years ago
|
||
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•15 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•15 years ago
|
Attachment #545796 -
Flags: review- → review?(ehsan)
Comment 4•15 years ago
|
||
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•15 years ago
|
||
Whiteboard: [cleanup] → [cleanup][fixed-in-fx-team]
| Assignee | ||
Comment 6•15 years ago
|
||
| Assignee | ||
Comment 7•15 years ago
|
||
| Assignee | ||
Comment 8•15 years ago
|
||
Status: ASSIGNED → RESOLVED
Closed: 15 years ago
Resolution: --- → FIXED
Whiteboard: [cleanup][fixed-in-fx-team] → [cleanup]
Target Milestone: --- → Firefox 8
Updated•10 years ago
|
Product: Firefox → Firefox Graveyard
Backout by clegnitto@mozilla.com:
https://hg.mozilla.org/releases/mozilla-release/rev/7099d3efac02
Backed out changeset f7599a2eb53a
You need to log in
before you can comment on or make changes to this bug.
Description
•