Last Comment Bug 526148 - Port Bug 525712 (Remove dead throbber-specific code from customizeToolbar.js, handle it in themes)
: Port Bug 525712 (Remove dead throbber-specific code from customizeToolbar.js,...
Status: RESOLVED FIXED
:
Product: SeaMonkey
Classification: Client Software
Component: Themes (show other bugs)
: Trunk
: All All
: -- normal (vote)
: seamonkey2.1a1
Assigned To: Philip Chee
:
Mentors:
Depends on: 525712
Blocks:
  Show dependency treegraph
 
Reported: 2009-11-02 20:24 PST by Philip Chee
Modified: 2009-12-11 14:05 PST (History)
3 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---


Attachments
Patch v1.0 SeaMonkey (2.20 KB, patch)
2009-11-29 06:32 PST, Philip Chee
neil: review-
Details | Diff | Splinter Review
Patch v1.1 LittleBig (3.74 KB, patch)
2009-11-30 08:30 PST, Philip Chee
neil: review+
Details | Diff | Splinter Review
Perverse approach (3.19 KB, patch)
2009-11-30 08:50 PST, neil@parkwaycc.co.uk
no flags Details | Diff | Splinter Review
Patch v1.2 Complex first [Checkin: Comment 16] (3.42 KB, patch)
2009-12-10 01:05 PST, Philip Chee
philip.chee: review+
neil: superreview+
Details | Diff | Splinter Review

Description Philip Chee 2009-11-02 20:24:44 PST
+++ This bug was initially created as a clone of Bug #525712 +++
Comment 1 Ian Neal 2009-11-07 10:23:01 PST
Whilst you are at it is it worth changing all the
<toolbaritem id="throbber-box"...>
  <button id="navigator-throbber"...>
</toolbaritem>
into
<toolbaritem id="navigator-throbber".../>
I've also noticed that /suite/mailnews/addrbook/addressbox.xul has a navigator-throbber that observes the element broadcaster_throbber that does not exist (broadcaster-throbber does but I cannot see anything that actually changes it).
/suite/mailnews/compose/messengercompose.xul has a navigator-throbber that observes the element broadcaster_throbber that does not exist (nor does broadcaster-throbber in this case either).
Comment 2 Philip Chee 2009-11-29 06:32:16 PST
Created attachment 414962 [details] [diff] [review]
Patch v1.0 SeaMonkey

Fix SeaMonkey throbber CSS following Firefox and Thunderbird.
Comment 3 Philip Chee 2009-11-29 06:33:26 PST
> Whilst you are at it is it worth changing all the
Separate bug for all those issues please.
Comment 4 neil@parkwaycc.co.uk 2009-11-29 12:32:05 PST
Comment on attachment 414962 [details] [diff] [review]
Patch v1.0 SeaMonkey

Sorry, this doesn't work with small icons.
Comment 5 Philip Chee 2009-11-29 19:49:14 PST
> Sorry, this doesn't work with small icons.
I didn't change anything that touched small icons. What do you mean "doesn't work with small icons" exactly?
Comment 6 neil@parkwaycc.co.uk 2009-11-30 01:08:53 PST
* Select "Use small icons"
* Customise toolbar

Would it help to change the existing "busy" rules to use toolbar > ?
Comment 7 Philip Chee 2009-11-30 01:55:40 PST
> * Select "Use small icons"
> * Customise toolbar
I don't see any change in the existing behaviour.

> Would it help to change the existing "busy" rules to use toolbar > ?
Good point. I'll test a new patch RSN.
Comment 8 Philip Chee 2009-11-30 08:30:24 PST
Created attachment 415151 [details] [diff] [review]
Patch v1.1 LittleBig

> Sorry, this doesn't work with small icons.
Fixed small icons issue.
Comment 9 neil@parkwaycc.co.uk 2009-11-30 08:50:45 PST
Created attachment 415156 [details] [diff] [review]
Perverse approach

Because customising toolbars removes the command attribute ;-)
Comment 10 neil@parkwaycc.co.uk 2009-11-30 08:51:55 PST
Comment on attachment 415151 [details] [diff] [review]
Patch v1.1 LittleBig

> window[chromehidden~="toolbar"] #navigator-throbber,
Can anyone remember what this does? I'm wondering whether it needs to work with toolbar customisation or whether we should just drop it completely.
Comment 11 Philip Chee 2009-11-30 10:13:13 PST
>> window[chromehidden~="toolbar"] #navigator-throbber,
> Can anyone remember what this does? I'm wondering whether it needs to work with
> toolbar customisation or whether we should just drop it completely.
I think it has something to do with full screen mode.
Comment 12 neil@parkwaycc.co.uk 2009-12-01 03:12:41 PST
Comment on attachment 415151 [details] [diff] [review]
Patch v1.1 LittleBig

... which of course isn't customisable.

Nit: for other rulesets I think we put the more complex rules first. This was probably to avoid changing the existing lines ;-)
Comment 13 Philip Chee 2009-12-01 03:21:58 PST
Comment on attachment 415151 [details] [diff] [review]
Patch v1.1 LittleBig

> Nit: for other rulesets I think we put the more complex rules first. This was
> probably to avoid changing the existing lines ;-)

Then eventually they get resorted in css clean-up drives. Or rather I file css clean up bugs and then forget to do them.
Comment 14 Philip Chee 2009-12-10 01:05:04 PST
Created attachment 416879 [details] [diff] [review]
Patch v1.2 Complex first
[Checkin: Comment 16]

> Nit: for other rulesets I think we put the more complex rules first. This was
> probably to avoid changing the existing lines ;-)

Fixed.
Comment 15 neil@parkwaycc.co.uk 2009-12-10 01:26:32 PST
Comment on attachment 416879 [details] [diff] [review]
Patch v1.2 Complex first
[Checkin: Comment 16]

Checked ;-)
Comment 16 Serge Gautherie (:sgautherie) 2009-12-11 14:05:18 PST
Comment on attachment 416879 [details] [diff] [review]
Patch v1.2 Complex first
[Checkin: Comment 16]


http://hg.mozilla.org/comm-central/rev/eef17e975072

Note You need to log in before you can comment on or make changes to this bug.