Port Bug 525712 (Remove dead throbber-specific code from customizeToolbar.js, handle it in themes)

RESOLVED FIXED in seamonkey2.1a1

Status

SeaMonkey
Themes
RESOLVED FIXED
8 years ago
8 years ago

People

(Reporter: Philip Chee, Assigned: Philip Chee)

Tracking

Trunk
seamonkey2.1a1

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(2 attachments, 2 obsolete attachments)

(Assignee)

Description

8 years ago
+++ This bug was initially created as a clone of Bug #525712 +++

Comment 1

8 years ago
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).
(Assignee)

Comment 2

8 years ago
Created attachment 414962 [details] [diff] [review]
Patch v1.0 SeaMonkey

Fix SeaMonkey throbber CSS following Firefox and Thunderbird.
Attachment #414962 - Flags: superreview?(neil)
Attachment #414962 - Flags: review?(neil)
(Assignee)

Comment 3

8 years ago
> Whilst you are at it is it worth changing all the
Separate bug for all those issues please.

Comment 4

8 years ago
Comment on attachment 414962 [details] [diff] [review]
Patch v1.0 SeaMonkey

Sorry, this doesn't work with small icons.
Attachment #414962 - Flags: superreview?(neil)
Attachment #414962 - Flags: review?(neil)
Attachment #414962 - Flags: review-
(Assignee)

Comment 5

8 years ago
> 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

8 years ago
* Select "Use small icons"
* Customise toolbar

Would it help to change the existing "busy" rules to use toolbar > ?
(Assignee)

Comment 7

8 years ago
> * 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.
(Assignee)

Comment 8

8 years ago
Created attachment 415151 [details] [diff] [review]
Patch v1.1 LittleBig

> Sorry, this doesn't work with small icons.
Fixed small icons issue.
Attachment #414962 - Attachment is obsolete: true
Attachment #415151 - Flags: review?(neil)

Comment 9

8 years ago
Created attachment 415156 [details] [diff] [review]
Perverse approach

Because customising toolbars removes the command attribute ;-)
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.
(Assignee)

Comment 11

8 years ago
>> 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 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 ;-)
Attachment #415151 - Flags: review?(neil) → review+
(Assignee)

Comment 13

8 years ago
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.
Attachment #415151 - Flags: superreview?
(Assignee)

Comment 14

8 years ago
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.
Attachment #415151 - Attachment is obsolete: true
Attachment #416879 - Flags: superreview?(neil)
Attachment #416879 - Flags: review+
Attachment #415151 - Flags: superreview?
Comment on attachment 416879 [details] [diff] [review]
Patch v1.2 Complex first
[Checkin: Comment 16]

Checked ;-)
Attachment #416879 - Flags: superreview?(neil) → superreview+
(Assignee)

Updated

8 years ago
Attachment #416879 - Attachment description: Patch v1.2 Complex first. r=neil → [for c-c checkin] Patch v1.2 Complex first. r=neil
(Assignee)

Updated

8 years ago
Keywords: checkin-needed
Comment on attachment 416879 [details] [diff] [review]
Patch v1.2 Complex first
[Checkin: Comment 16]


http://hg.mozilla.org/comm-central/rev/eef17e975072
Attachment #416879 - Attachment description: [for c-c checkin] Patch v1.2 Complex first. r=neil → Patch v1.2 Complex first [Checkin: Comment 16]
Status: ASSIGNED → RESOLVED
Last Resolved: 8 years ago
Resolution: --- → FIXED
Target Milestone: --- → seamonkey2.1a1
Keywords: checkin-needed
You need to log in before you can comment on or make changes to this bug.