914258 - Backout the patch for bug 887515 and its dependencies from Firefox 25

Status: VERIFIED FIXED

(Firefox :: Menus, defect)

Description

[Jared Wein [:jaws] (please needinfo? me)]

The amount of outstanding regressions based on the feature introduced in bug 887515 is too high and some of the fixes may be best if a new API was introduced (bug 895436).

This bug is to backout the patch for bug 887515 and its follow-up bugs from Firefox 25.

Comment 1

[Jared Wein [:jaws] (please needinfo? me)]

Attachment: backout-35934f1b4a45

Comment 2

[Jared Wein [:jaws] (please needinfo? me)]

Attachment: backout-fdb2be03094d

Comment 3

[Jared Wein [:jaws] (please needinfo? me)]

Attachment: backout-b94553178dfe

I left the uuid in nsISessionStore.idl unchanged from mozilla-aurora tip since there have been other changes to the file that are not being backed out.

Comment 4

[Jared Wein [:jaws] (please needinfo? me)]

Comment on attachment 801995 [details] [diff] [review]
backout-35934f1b4a45

[Approval Request Comment]
Bug caused by (feature/regressing bug #): Bug 887515 has too many regressions and not enough time to fix them all on Aurora
User impact if declined: Various bugs with undoing closing tabs.
Testing completed (on m-c, etc.): locally
Risk to taking this patch (and alternatives if risky): none expected, simple backout
String or IDL/UUID changes made by this patch: these patches had introduced a new string, but never removed the old one. Since this is a backout, there is string removal but the strings that are removed shouldn't mess up localization since the string that will be used in its place was and will still be around.

Comment 5

[Jared Wein [:jaws] (please needinfo? me)]

Comment on attachment 801997 [details] [diff] [review]
backout-fdb2be03094d

See above aurora-approval request.

Comment 6

[Jared Wein [:jaws] (please needinfo? me)]

Comment on attachment 801998 [details] [diff] [review]
backout-b94553178dfe

See above aurora-approval request.

Comment 7

[Jared Wein [:jaws] (please needinfo? me)]

remote:   https://hg.mozilla.org/releases/mozilla-aurora/rev/1bb6b1468e7e
remote:   https://hg.mozilla.org/releases/mozilla-aurora/rev/4aa851a2fc9c
remote:   https://hg.mozilla.org/releases/mozilla-aurora/rev/d476bbada7f5

Comment 8

[u279076]

Jared, is there something QA needs to test here?

Comment 9

[Jared Wein [:jaws] (please needinfo? me)]

(In reply to Anthony Hughes, Mozilla QA (:ashughes) from comment #8)
> Jared, is there something QA needs to test here?

Basically, "Undo Close Tab" should work the same in Firefox 25 as it does in Firefox 24. That is:
1) close a tab and then right-click on another tab and choose "Undo Close Tab". The most recently closed tab should be reopened.
2) right-click on a tab and choose "Close Other Tabs". Then right-click on the tab and choose "Undo Close Tab", and only one of the recently closed tabs should reopen. This menuitem will have to be clicked multiple times (once for each tab that got closed) to reopen all the closed tabs.

Comment 10

[u279076]

Thanks Jared, flagging for verification.

Comment 11

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

(In reply to Jared Wein [:jaws] from comment #3)
> I left the uuid in nsISessionStore.idl unchanged from mozilla-aurora tip
> since there have been other changes to the file that are not being backed
> out.

In general, this isn't a good idea. Any binary-incompatible changes (e.g. removing methods) to an IDL must be paired with a change to the IID, otherwise clients can end up crashing.

If the interface definition ends up matching a previous state, it's OK to re-use that previous state's IID, but that doesn't appear to be the case here. So we should bump the IID for nsISessionStore on Aurora, I think.

(It may not matter in practice, I doubt somewhat that there are binary callers to this interface. But it's relatively low cost, so I think worth doing anyways.)

Comment 12

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

Good call on the backout, btw; good that we didn't lose track of this.

Comment 13

[Jared Wein [:jaws] (please needinfo? me)]

Attachment: Followup patch to rev the UUID for Fx25

Comment 14

[Jared Wein [:jaws] (please needinfo? me)]

Comment on attachment 805450 [details] [diff] [review]
Followup patch to rev the UUID for Fx25

[Approval Request Comment]
Bug caused by (feature/regressing bug #): followup for this bug
User impact if declined: potential binary incompatibility due to incorrect UUID
Testing completed (on m-c, etc.): locally
Risk to taking this patch (and alternatives if risky): none expected
String or IDL/UUID changes made by this patch: none

This should have been done with the aurora-uplifted patches, but those have now made their way to beta so this will need to be uplifted to beta.

Comment 15

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

Too bad we missed this for beta 1, I guess I should have approved for Aurora when I r+ed. We should take this now on beta for b2, I think.

Comment 16

[Alex Keybl [:akeybl]]

Comment on attachment 805450 [details] [diff] [review]
Followup patch to rev the UUID for Fx25

Because we're UUID frozen on beta, and there likely weren't any major clients of the new UUID yet, we're going to leave it as is.

Comment 17

[Bogdan Maris, Desktop Test Engineering]

Mozilla/5.0 (X11; Ubuntu; Linux i686; rv:25.0) Gecko/20100101 Firefox/25.0
Mozilla/5.0 (Macintosh; Intel Mac OS X 10.8; rv:25.0) Gecko/20100101 Firefox/25.0
Mozilla/5.0 (Windows NT 5.1; rv:25.0) Gecko/20100101 Firefox/25.0

Verified as fixed on Firefox 25 beta 3 (buildID: 20130926170421).