bug 748160 regressed this, the placeholder was observing Tools:Downloads, the patch made it use the command handler, plus on another patch landed at the same time we started replacing the button with the placeholder regardless (we were keeping the placeholder forever previously). STR: 1. set browser.downloads.useToolkitUI = true (or just use beta/aurora where it's already set) 2. put the downloads button on the toolbar 3. click on it Expected: The old DM window opens Currently: The new panel opens

This bug has been found on Mozillazine forums, patrickjdempsey found this regression-range: http://hg.mozilla.org/mozilla-central/pushloghtml?changeset=b3cce81fef1a

this is a simple patch that may be used. My concern is whether we should do more, like avoiding replacing the placeholder or such, but actually that could be just a waste of time for this simple fix. The fix for this bug will have to be backported to Aurora and Beta, since feature disable is broken currently.

Comment on attachment 678729 [details] [diff] [review] patch v1.0 Regression indeed.

(In reply to Marco Bonardo [:mak] from comment #2) > My concern is whether we should do more, like avoiding replacing the > placeholder or such, but actually that could be just a waste of time for > this simple fix. I think like you here. Let's see how difficult it is to avoid replacing the placeholder with the indicator, but this fix should be done regardless.

At this stage in the 17 beta cycle, we need to look at backing out the regressing bug and fixing this properly on Aurora instead. Can someone prepare a backout patch for bug 748160 that returns us to a known good state?

(In reply to Lukas Blakk [:lsblakk] from comment #6) > At this stage in the 17 beta cycle, we need to look at backing out the > regressing bug and fixing this properly on Aurora instead. Can someone > prepare a backout patch for bug 748160 that returns us to a known good state? the backout is too risky imo, we coded on top of it, this patch is 10 times less risky than the backout.

(In reply to Marco Bonardo [:mak] from comment #7) > (In reply to Lukas Blakk [:lsblakk] from comment #6) > > At this stage in the 17 beta cycle, we need to look at backing out the > > regressing bug and fixing this properly on Aurora instead. Can someone > > prepare a backout patch for bug 748160 that returns us to a known good state? > > the backout is too risky imo, we coded on top of it, this patch is 10 times > less risky than the backout. Sounds like we need an uplift nomination here then, once qa has done verification on trunk, so this can get into our final beta 17.

Comment on attachment 678729 [details] [diff] [review] patch v1.0 [Approval Request Comment] Bug caused by (feature/regressing bug #): bug 748160 User impact if declined: The downloads button opens the panel even if this feature is disabled, it should keep opening the old DM Testing completed (on m-c, etc.): m-c Risk to taking this patch (and alternatives if risky): It's basically a oneliner, so risk is limited. Alternative is to find all the downloads panel bugs that landed in the meanwhile and backout all of them, there's risk to miss something though, making things worse. String or UUID changes made by this patch: none

Verified on Windows 7 64-bit, Ubuntu 12.04 and Mac OS X 10.8: Mozilla/5.0 (Windows NT 6.1; WOW64; rv:19.0) Gecko/19.0 Firefox/19.0 Mozilla/5.0 (X11; Linux i686; rv:19.0) Gecko/19.0 Firefox/19.0 Mozilla/5.0 (Macintosh; Intel Mac OS X 10.8; rv:19.0) Gecko/19.0 Firefox/19.0 Build ID: 20121107030713 Verified that the the downloads button doesn't open the panel if it's disabled on the latest Nightly.

https://hg.mozilla.org/releases/mozilla-aurora/rev/34b009e46fd1 https://hg.mozilla.org/releases/mozilla-beta/rev/25b230afe821

Reproducible in 17.0b5 Linux-x86_64.

(In reply to Aleksej [:Aleksej] from comment #13) > Reproducible in 17.0b5 Linux-x86_64. b05 has been released just before this fix, so that's expected.

(In reply to Aleksej [:Aleksej] from comment #13) > Reproducible in 17.0b5 Linux-x86_64. Please try the most recent beta build from http://ftp.mozilla.org/pub/mozilla.org/firefox/tinderbox-builds/mozilla-beta-linux/ to see if the current code that is about to become beta 6 is fixed.

Verified fixed with https://ftp.mozilla.org/pub/mozilla.org/firefox/tinderbox-builds/mozilla-beta-linux/1352721280/firefox-17.0.en-US.linux-i686.tar.bz2

Verified as fixed on Firefox 17 beta 6 - the downloads button doesn't open the downloads panel if is disabled. Verified on Windows 7, Ubuntu 12.04 and Mac OS X 10.8: Mozilla/5.0 (Windows NT 6.1; rv:17.0) Gecko/17.0 Firefox/17.0 Mozilla/5.0 (X11; Linux i686; rv:17.0) Gecko/17.0 Firefox/17.0 Mozilla/5.0 (Macintosh; Intel Mac OS X 10.8; rv:17.0) Gecko/17.0 Firefox/17.0

Verified as fixed on Firefox 18 beta 2. This issue in no more reproducing. Mozilla/5.0 (Windows NT 6.1; WOW64; rv:18.0) Gecko/20100101 Firefox/18.0 (20121128060531) Mozilla/5.0 (X11; Linux i686; rv:18.0) Gecko/20100101 Firefox/18.0 (20121128060531) Mozilla/5.0 (Macintosh; Intel Mac OS X 10.8; rv:18.0) Gecko/20100101 Firefox/18.0 (20121128060531) Please someone with permission set status for firefox18 as verified.

(In reply to MarioMi from comment #18) > Verified as fixed on Firefox 18 beta 2. This issue in no more reproducing. Same results for FF 19b2