1274533 - Move legacy behaviour change from text-link binding to handle-xul-text-link observer

Status: RESOLVED FIXED

(Firefox :: General, defect)

Description

[:Gijs (he/him)]

(In reply to tabmix.onemen from bug 1268943 comment #23)
> The "Preserve legacy behavior" part in text.js#text-link prevents any one
> that observes "handle-xul-text-link" from getting the real event data.
> 
> >          if (!shiftKey && !altKey) {
> >            // Preserve legacy behavior of non-modifier left-clicks
> >            // opening in a new selected tab.
> >            let {AppConstants} =
> >              Components.utils.import("resource://gre/modules/AppConstants.jsm", {});
> >            if (AppConstants.platform == "macosx") {
> >              metaKey = true;
> >            } else {
> >              ctrlKey = true;
> >            }
> >          }
> 
> Tab mix add option to inverse the behavior of
> "browser.tabs.loadInBackground" when user middle-click on links, to one can
> click open the link and stay on the current page.
> 
> Is it possible to move the "Preserve legacy behavior" to
> "handle-xul-text-link" observer
> >        if (!linkHandled.data) {
> >          let win = RecentWindow.getMostRecentBrowserWindow();
> >          if (win) {
> >            data = JSON.parse(data);
> >            let where = win.whereToOpenLink(data);
> >+           // Preserve legacy behavior of non-modifier left-clicks
> >+           // opening in a new selected tab.
> >+           if (where == "current") {
> >+             where = "tab";
> >+           }
> >            win.openUILinkIn(data.href, where);
> >            linkHandled.data = true;
> >          }
> >        }

This seems reasonable to me.

Comment 1

[:Gijs (he/him)]

Attachment: MozReview Request: Bug 1274533 - change legacy behaviour in observer rather than overwriting key status in data, r?jaws

Review commit: https://reviewboard.mozilla.org/r/54236/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/54236/

Comment 2

[tabmix.onemen]

>+            let where = win.whereToOpenLink(data);
>+            Cu.reportError(where);
>+            if (where == "current") {
>+              where = "tab";
>+            }

"Cu.reportError(where)" ?????

Comment 3

[:Gijs (he/him)]

Comment on attachment 8754774 [details]
MozReview Request: Bug 1274533 - change legacy behaviour in observer rather than overwriting key status in data, r?jaws

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/54236/diff/1-2/

Comment 4

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

Comment on attachment 8754774 [details]
MozReview Request: Bug 1274533 - change legacy behaviour in observer rather than overwriting key status in data, r?jaws

https://reviewboard.mozilla.org/r/54236/#review50978

::: browser/components/nsBrowserGlue.js:383
(Diff revision 2)
> +            if (where == "current") {
> +              where = "tab";
> +            }

Move the comment over from text.xml to explain why `where` is getting changed here.

Comment 5

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/c2a4ec679a40

Comment 6

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/c2a4ec679a40