In the Add-ons Manager, Experiments tab: experiments are marked with the "striped" background which means they are marked for removal. Also, if you double click on it, you enter into the detailed view which also displays text telling the user about the pending removal.

I'm not sure if we mind the striped background in the addon list view, but we definitely need to hide the "pending notification" in the detail view around here: http://hg.mozilla.org/mozilla-central/annotate/37f08ddaea48/toolkit/mozapps/extensions/content/extensions.css#l243 Due to how we integrate with the addon manager, all experiment addons have "pending disable", which is why we hide the pending notification elements of the addon manager UI. Obviously i missed a spot here.

Comment on attachment 8447905 [details] [diff] [review] Fix pending in detail view r- on the basis that this is an empty patch.

Post-lunch this patch looks a little less empty.

Added to Iteration 33.2

Added to Iteration 33.2

Comment on attachment 8447936 [details] [diff] [review] Fix pending in detail view Review of attachment 8447936 [details] [diff] [review]: ----------------------------------------------------------------- (In reply to Georg Fritzsche [:gfritzsche] from comment #2) > Due to how we integrate with the addon manager, all experiment addons have > "pending disable", which is why we hide the pending notification elements of > the addon manager UI. Obviously i missed a spot here. Wait, what? Why? That's new to me, and seems broken and the thing we should fix.

*yoink!* Patch coming up, following IRC discussion on this.

Comment on attachment 8448623 [details] [diff] [review] Patch v2 Review of attachment 8448623 [details] [diff] [review]: ----------------------------------------------------------------- ::: toolkit/mozapps/extensions/internal/XPIProvider.jsm @@ +6645,5 @@ > // operations > return AddonManager.PENDING_UNINSTALL; > } > > + // Extensions have an intentional inconsistancy between what the DB says is nit: s/inconsistancy/inconsistency/ @@ +6653,5 @@ > + if (aAddon.active && isAddonDisabled(aAddon)) > + pending |= AddonManager.PENDING_DISABLE; > + else if (!aAddon.active && !isAddonDisabled(aAddon)) > + pending |= AddonManager.PENDING_ENABLE; > + } I wish we could refactor AddonInternal so that there were separate implementations for theme / experiment / extension / ... so that we don't have to spread "if addon.type ..." all over the XPI provider. (I'm a http://www.antiifcampaign.com/ sympathiser...)

(In reply to :Irving Reid from comment #11) > I wish we could refactor AddonInternal so that there were separate > implementations for theme / experiment / extension / ... so that we don't > have to spread "if addon.type ..." all over the XPI provider. (I'm a > http://www.antiifcampaign.com/ sympathiser...) I've been thinking the same thing. I'd already filed bug 986306, but doing it via refactoring into separate objects with a base implementation would also work.

Hi Florin, can a QA contact be assigned for verification.

Kamil can you handle this since it's related to experiments?

Went through verification using the following build: * firefox-33.0a1.en-US.win32 [BuildID: 20140707030202] ** http://ftp.mozilla.org/pub/mozilla.org/firefox/nightly/2014-07-07-03-02-02-mozilla-central/ - ensured that active experiment tabs under about:addons are not using stripped background(s) - ensured that the detailed view doesn't display a message indicating that the experiment is going to be removed on restart - ensured both "Learn More" & "Telemetry Settings" buttons are still working - ensured that the "more" button on the experiment tab takes the user into detailed view - ensured that double clicking on an active/removed experiment under about:addons displays the detailed view - ensured that there's no UI/UX issues due to removing the stripped background - moved the machines time ahead by one day and ensured that the "x days remaining" is being counted correctly - ensured that the "x days remaining" from the experiment tab under about:addons matches the information displayed under the detailed view - ensured that the experiment expires correctly - ensured that the experiment is removed when "Remove" on either the experiment tab or on the detailed view is selected - ensured that the "undo" button is working correctly when removing an experiment - ensured that the "undo" message is using the correct stripped background (pending removal) - ensured that the background for the tabs/detailed view are grayed out when the experiment is removed/expired

Should this be uplifted to beta?? We currently have the search experiment on beta and are about to land the translation experiments for the vi, pl and tr locales. I ran into this when I was testing the new translation experiment in bug #1041598

Benjamin is away. Too bad we missed Aurora uplift. I'm not sure if this is something we'll still take on beta given the risk/win here. [Tracking Requested - why for this release]: Affected release and we ended up doing a few experiments on 32.

Comment on attachment 8448623 [details] [diff] [review] Patch v2 Approval Request Comment [Feature/regressing bug #]: Telemetry experiments. [User impact if declined]: Experiments incorrectly/misleadingly shown as "pending removal" in Addon Manager. [Describe test coverage new/current, TBPL]: Baked fine for a while on m-c with active experiments. Automated test coverage. [Risks and why]: Low-risk - only simple branching to avoid setting the "pending disable" state on experiments. [String/UUID change made/needed]: None.

Comment on attachment 8448623 [details] [diff] [review] Patch v2 Given that we're currently running 2 experiments on beta, I'm going to say beta+. However, I would like to see this land before beta2, which goes to build early Monday, July 28. Please land before July 28.

For each of the experiments that are currently on beta, I went through the test cases in comment # 17 and ensured that the original issue wasn't occurring: Search Experiment: [Passed] Build: http://ftp.mozilla.org/pub/mozilla.org/firefox/releases/32.0b6/win32/en-US/ OOPP container unload timeout tester: [Passed] Build: http://ftp.mozilla.org/pub/mozilla.org/firefox/releases/32.0b6/win32/en-US/ Automatic Translation - PL: [Passed] Build: http://ftp.mozilla.org/pub/mozilla.org/firefox/releases/32.0b6/win32/pl/ Automatic Translation - TR: [Passed] Build: http://ftp.mozilla.org/pub/mozilla.org/firefox/releases/32.0b6/win32/tr/ Automatic Translation - VI: [Passed] Build: http://ftp.mozilla.org/pub/mozilla.org/firefox/releases/32.0b6/win32/vi/