887950 - Test failure with bug 653881 in the notificationbar code

Status: RESOLVED WONTFIX

(Firefox :: Theme, defect)

Description

[Blake Kaplan (:mrbkap) (inactive)]

Attachment: Proposed patch

This is very similar to several of the dependencies on bug 653881 -- the OSX theme for the notification bar gets tripped up by the additional xbl:children elements added as a result of that bug. The patch is trivial. I'd appreciate a review asap. We're investigating some solutions to make this situation less whack-a-mole-y, but for the moment, I'd like to simply get the easy fixes in.

Comment 1

[Markus Stange [:mstange]]

Comment on attachment 768505 [details] [diff] [review]
Proposed patch

I'm basing my review on the similarity to bug 875165.

Comment 2

[:Gavin Sharp [email: [e-mail]]]

Won't you also need to fix almost all of the other hits in
http://mxr.mozilla.org/mozilla-central/search?string=.button-text

? The compat problems at this point look too large for this landing to be feasible.

Comment 3

[Blake Kaplan (:mrbkap) (inactive)]

(In reply to :Gavin Sharp (use [e-mail] for email) from comment #2)
> Won't you also need to fix almost all of the other hits in
> http://mxr.mozilla.org/mozilla-central/search?string=.button-text
> 
> ? The compat problems at this point look too large for this landing to be
> feasible.

See bug 653881 comment 98. I plan to land both patches.

Comment 4

[:Gavin Sharp [email: [e-mail]]]

Is it really worth landing this patch (and all the others like it) if you're going to land that one?

Comment 5

[Blake Kaplan (:mrbkap) (inactive)]

It's a matter of taste, I suppose. I prefer the more explicit style, so I'd prefer to land both patches but if you or mstange prefer, I can not land this patch (wchen confirmed yesterday that the other patch fixes this bug independently of this patch).

Comment 6

[Markus Stange [:mstange]]

I don't have that much experience with bindings so I don't have an opinion on this.

Comment 7

[Blake Kaplan (:mrbkap) (inactive)]

I went ahead and landed this. Gavin, if you feel strongly that I should back out, let me know.

https://hg.mozilla.org/integration/mozilla-inbound/rev/989747f479ee

Comment 8

[Phil Ringnalda (:philor)]

https://hg.mozilla.org/mozilla-central/rev/989747f479ee

Comment 9

[:Gavin Sharp [email: [e-mail]]]

Comment on attachment 768505 [details] [diff] [review]
Proposed patch

It seems confusing to introduce inconsistencies in the tree about which type of selector should be used - if we're going to keep the old style working, it seems better to avoid changing anything that doesn't need changing.

Comment 10

[Markus Stange [:mstange]]

This argument sounds very convincing to me. I'm now also in favor of a backout.

Comment 11

[Blake Kaplan (:mrbkap) (inactive)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/86ece8e34a93

Sure.

Comment 12

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/86ece8e34a93