1296863 - Stop disabling the "New Tab" command in popups

Status: RESOLVED FIXED

(Firefox :: Tabbed Browser, defect, P3)

Description

[Dão Gottwald [:dao]]

Attachment: patch

This was disabled in bug 470051 because it broke the popup window. Since then, we've fixed BrowserOpenTab and openUILinkIn to do the right thing here.

This way we can also simplify the code from bug 528005 by letting <key id="key_newNavigatorTab"> call BrowserOpenTab directly but without the event.

Comment 1

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/d35635870180
Stop disabling the "New Tab" command in popups. r=gijs

Comment 2

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/2beb0d486fc6
followup: don't break reserved-ness of accel-t to open tabs, rs=bustage

Comment 3

[:Gijs (he/him)]

I pushed a followup to fix a breaking test that checked that accel-t was still "reserved" by the browser, which is controlled by the <command> rather than the <key> (which I don't really follow, but OK...) so I added a second <command> that the key handler invokes and made that reserved=true (too). Let me know if this makes sense (thinking about it, maybe we can remove the reserved keyword off the original command element? Really, these attributes should be on the keys rather than the commands, IMO...).

Comment 4

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

Followup does appear to have fixed the failures I was seeing, thanks Gijs :)

Comment 5

[Dão Gottwald [:dao]]

(In reply to :Gijs Kruitbosch from comment #3)
> I pushed a followup to fix a breaking test that checked that accel-t was
> still "reserved" by the browser, which is controlled by the <command> rather
> than the <key> (which I don't really follow, but OK...) so I added a second
> <command> that the key handler invokes and made that reserved=true (too).

Thanks.

> Let me know if this makes sense (thinking about it, maybe we can remove the
> reserved keyword off the original command element? Really, these attributes
> should be on the keys rather than the commands, IMO...).

Yes, we should move them to the key elements.

Comment 6

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/d35635870180
https://hg.mozilla.org/mozilla-central/rev/2beb0d486fc6