Closed Bug 407655 Opened 17 years ago Closed 17 years ago

Reorganize download context menus

Categories

(Toolkit :: Downloads API, defect)

defect
Not set
normal

Tracking

()

VERIFIED FIXED
mozilla1.9beta3

People

(Reporter: Mardak, Assigned: Mardak)

References

Details

Attachments

(1 file, 2 obsolete files)

I know there's bug 403703, etc for making the menus match the latest spec. This bug just gets things ready to match the latest spec with what's already implemented (i.e., everything but "go to source" bug 403674).
Attached patch v1 (obsolete) (deleted) — Splinter Review
Menu ordering per attachment 288692 [details] and bug 397655 comment #34. Note, there's a cancel for scanning.. perhaps we should handle that..
Assignee: nobody → edilee
Status: NEW → ASSIGNED
Attachment #292373 - Flags: review?(comrade693+bmo)
Depends on: 407656
There are already filed bugs for each of these issues. Have a look at the blocking list of bug 403674. So how to proceed?
As mentioned in comment #0, this is just reorganizing the existing menu items before bug 403674. Doing this before bug 403674 will keep the "blame" around for at least bug 403674 instead of reorganizing afterwards.
Comment on attachment 292373 [details] [diff] [review] v1 >diff --git a/toolkit/mozapps/downloads/content/downloads.js > var gContextMenus = [ > // DOWNLOAD_DOWNLOADING Bug 403704 > // DOWNLOAD_FINISHED Bug 403703 > // DOWNLOAD_FAILED > // DOWNLOAD_CANCELED Bug 403709 > // DOWNLOAD_PAUSED Bug 403707 Only the following states are not covered by an already existing bug: > // DOWNLOAD_QUEUED > // DOWNLOAD_BLOCKED > // DOWNLOAD_SCANNING > // DOWNLOAD_DIRTY
The main reason why I didn't use those bugs is that those are to make the menus match the latest spec. This bug doesn't make it match the latest spec but gets it closer. stephend mentioned that those bugs are mainly to help keep things organized for testing. Additionally, those bugs depend on bug 403674 which this bug blocks, but this bug would be blocking each of bug 403704, etc.. and you can't make that cycle in bugzilla.
Attached patch v1.1 (obsolete) (deleted) — Splinter Review
Removed retry for blocked and dirty downloads (and their separators) From bug 397655 comment #38.. in progress: 1-2 actions followed by a separator, open containing, separator, go to download page, copy download link queued: cancel downloading: pause, cancel paused: resume, cancel scanning: cancel not in progress: 0-2 actions followed by a separator, go to download page, copy download link, separator, remove from list, clear list finished: open, open containing canceled: retry failed: retry blocked: <no action> dirty: <no action>
Attachment #292373 - Attachment is obsolete: true
Attachment #292446 - Flags: review?(comrade693+bmo)
Attachment #292373 - Flags: review?(comrade693+bmo)
Bug 392097 fixes inProgress to report scanning as a valid inProgress state.. Without that, the cancel menu item for scanning will just be gray.
Depends on: 392097
Comment on attachment 292446 [details] [diff] [review] v1.1 >+ ["menuitem_copyLocation", >+ "menuseparator", >+ "menuitem_removeFromList", >+ "menuitem_clearList"] nit: let's format the blocks like so: [ "menuitem_*" ",menu*" ] this way, adding anything ever won't mess up blame. >- <menuseparator id="menuseparator_copy_location"/> >+ <menuseparator id="menuseparator"/> why the change? r=sdwilsh with these addressed.
Attachment #292446 - Flags: review?(comrade693+bmo) → review+
Also no cancel for downloads in the scanning state.
(In reply to comment #8) > (From update of attachment 292446 [details] [diff] [review]) > >+ ["menuitem_copyLocation", > >+ "menuseparator", > >+ "menuitem_removeFromList", > >+ "menuitem_clearList"] > nit: let's format the blocks like so: > [ > "menuitem_*" > ",menu*" > ] > this way, adding anything ever won't mess up blame. I'm guessing you meant.. var gCont = [ // DOWN.. [ "menu.." , "menu.." ], .. ]; ? See spacing and comma.. should the state separator comma be on the next group like.. [ "" ] , [ ] ? > >- <menuseparator id="menuseparator_copy_location"/> > >+ <menuseparator id="menuseparator"/> > why the change? Because otherwise I would have needed to add in more menu separators with different names. The menu items get cloned anyway, so there's no difference between the copy_location separator and other ones like actions and referrer separators.
(In reply to comment #10) > ? See spacing and comma.. should the state separator comma be on the next group > like.. > [ > "" > ] > , [ > ] > ? I wouldn't bother with that one. Blame isn't so important in that case.
Attached patch v1.2 (deleted) — Splinter Review
(In reply to comment #8) > [ > "menuitem_*" > ",menu*" > ] > this way, adding anything ever won't mess up blame. Shuffled.. except the first item can still cause blame loss. (In reply to comment #9) > Also no cancel for downloads in the scanning state. Removed (and the menuseparator) r+ sdwilsh
Attachment #292446 - Attachment is obsolete: true
Attachment #293093 - Flags: review+
Attachment #293093 - Flags: approval1.9?
No depends on bug 392097 because we don't have cancel for scanning.
No longer depends on: 392097
Blocks: 408351
Comment on attachment 293093 [details] [diff] [review] v1.2 a=beltzner for 1.9
Attachment #293093 - Flags: approval1.9? → approval1.9+
Checking in toolkit/mozapps/downloads/content/downloads.js; /cvsroot/mozilla/toolkit/mozapps/downloads/content/downloads.js,v <-- downloads.js new revision: 1.111; previous revision: 1.110 done Checking in toolkit/mozapps/downloads/content/downloads.xul; /cvsroot/mozilla/toolkit/mozapps/downloads/content/downloads.xul,v <-- downloads.xul new revision: 1.37; previous revision: 1.36 done
Status: ASSIGNED → RESOLVED
Closed: 17 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 3 M11
Blocks: 403703
Blocks: 403704
Blocks: 403707
Blocks: 403709
Since this bug and all dependent bugs are fixed, verified FIXED with Mozilla/5.0 (Windows; U; Windows NT 6.0; en-US; rv:1.9b3pre) Gecko/2007121802 Minefield/3.0b3pre Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.4; en-US; rv:1.9b3pre) Gecko/2007121801 Minefield/3.0b3pre and Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.9b3pre) Gecko/2007121802 Minefield/3.0b3pre I'll be updating Litmus soon to reflect the (final? :-P) revision.
Status: RESOLVED → VERIFIED
Product: Firefox → Toolkit
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: