Closed Bug 873518 Opened 12 years ago Closed 11 years ago

Remove the activity indicator and old cut/copy/paste buttons from the palette

Categories

(Firefox :: Toolbars and Customization, defect)

defect
Not set
normal

Tracking

()

RESOLVED FIXED
Firefox 28

People

(Reporter: mconley, Assigned: Gijs)

References

(Blocks 1 open bug)

Details

(Whiteboard: [Australis:M6])

Attachments

(3 files)

The status indicator, or "throbber", is not going to be included in the list of widgets that are customizable.
Summary: Remove the throbber from the palette, and migrate it out of currentsets. → Remove the activity indicator from the palette, and migrate it out of currentsets.
You shouldn't have to migrate anything, customization code should gracefully handle IDs pointing to items that don't exist anymore.
(In reply to Dão Gottwald [:dao] from comment #1) > You shouldn't have to migrate anything, customization code should gracefully > handle IDs pointing to items that don't exist anymore. This.
Assignee: nobody → gijskruitbosch+bugs
Whiteboard: [Australis:M?] → [Australis:M6]
Summary: Remove the activity indicator from the palette, and migrate it out of currentsets. → Remove the activity indicator and old cut/copy/paste buttons from the palette
Status: NEW → ASSIGNED
Attached patch Part 1: remove throbber (deleted) — Splinter Review
Attachment #754752 - Flags: review?(jaws)
I haven't adapted the toolbar sprite image for this bug. I'm not sure if we'd consider the resulting churn worth it?
Attachment #754754 - Flags: review?(jaws)
Comment on attachment 754752 [details] [diff] [review] Part 1: remove throbber > else if (aStateFlags & nsIWebProgressListener.STATE_STOP) { > // This (thanks to the filter) is a network stop or the last >- // request stop outside of loading the document, stop throbbers >- // and progress bars and such >+ // request stop outside of loading the document, stop >+ // progress bars and such Please leave this comment alone. The throbber example remains meaningful as to what kind of UI should be updated here. Whether we do or do not have a global throbber doesn't matter much for the sake of giving an example. Note that we no longer have a progress bar either.
Found this trying to see if there were bits about our current code that break when the placements array contains non-existing widgets.
Attachment #754796 - Flags: review?(jaws)
Comment on attachment 754752 [details] [diff] [review] Part 1: remove throbber Review of attachment 754752 [details] [diff] [review]: ----------------------------------------------------------------- r=me As Dao mentioned, we can leave that comment unchanged. ::: browser/themes/osx/browser.css @@ -2140,5 @@ > - list-style-image: url("chrome://global/skin/icons/loading_16.png"); > -} > - > -#wrapper-navigator-throbber > #navigator-throbber { > - list-style-image: url("chrome://global/skin/icons/notloading_16.png"); It looks like notloading_16.png will be unused after this patch. We can probably just remove it as well. Out of curiousity, what happened on osx when the throbber wasn't in the busy state? From the looks of it, it seems like no list-style-image would be supplied.
Attachment #754752 - Flags: review?(jaws) → review+
(In reply to Jared Wein [:jaws] from comment #7) > Comment on attachment 754752 [details] [diff] [review] > Part 1: remove throbber > > Review of attachment 754752 [details] [diff] [review]: > ----------------------------------------------------------------- > > r=me > > As Dao mentioned, we can leave that comment unchanged. > > ::: browser/themes/osx/browser.css > @@ -2140,5 @@ > > - list-style-image: url("chrome://global/skin/icons/loading_16.png"); > > -} > > - > > -#wrapper-navigator-throbber > #navigator-throbber { > > - list-style-image: url("chrome://global/skin/icons/notloading_16.png"); > > It looks like notloading_16.png will be unused after this patch. We can > probably just remove it as well. Huh, seems you're right. I checked for loading_16.png, that's still used in a couple of places, so I figured this would be the same. > Out of curiousity, what happened on osx when the throbber wasn't in the busy > state? From the looks of it, it seems like no list-style-image would be > supplied. Correct. (on 22b, at least)
Comment on attachment 754754 [details] [diff] [review] Part 2: Remove cut/copy/paste buttons Review of attachment 754754 [details] [diff] [review]: ----------------------------------------------------------------- We can update Toolbar.png later when we get images for the new widgets added to it. ::: browser/base/content/browser.js @@ +3363,5 @@ > // The UI is visible if the Edit menu is opening or open, if the context menu > // is open, or if the toolbar has been customized to include the Cut, Copy, > // or Paste toolbar buttons. > + // XXXgijs: this needs to be updated with the unified cut/copy/paste toolbarbutton > + // once that is customizable! Please include a reference to bug 870901 here.
Attachment #754754 - Flags: review?(jaws) → review+
Attachment #754796 - Flags: review?(jaws) → review+
All pushed to UX, notloading_16.png removed (also from jar.mn), throbber-referencing comment intact, bug 870901 mentioned, push URLs: https://hg.mozilla.org/projects/ux/rev/4a5aa012dee7 https://hg.mozilla.org/projects/ux/rev/a17f84ab819e https://hg.mozilla.org/projects/ux/rev/5b35c6c3e9e0
Whiteboard: [Australis:M6] → [Australis:M6][fixed-in-ux]
Status: ASSIGNED → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
Whiteboard: [Australis:M6][fixed-in-ux] → [Australis:M6]
Target Milestone: --- → Firefox 28
Tested on :--- User Agent : Mozilla/5.0 (Windows NT 6.1; Win64; x64; rv:28.0) Gecko/20100101 Firefox/28.0 Version : 28.0a1 Platform Tested : "Windows 7 64 bit" Status : Not completely fixed. Activity indicator is removed, but,not the cut/copy/paste.
(In reply to ganesh_sahai1 from comment #12) > Status : Not completely fixed. Activity indicator is removed, but,not the > cut/copy/paste. Hello, this bug is referring to individual cut, copy and paste buttons and you are likely referring to the unified cut/copy/paste set which are expected.
I am not seeing any cut, copy and paste buttons. Unified cut/copy/paste is there which as per the previous comment seems to be expected. So,given this state, this bug is fixed now.
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: