Closed Bug 1274533 Opened 5 years ago Closed 5 years ago

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

Categories

(Firefox :: General, defect)

defect
Not set
normal

Tracking

()

RESOLVED FIXED
Firefox 49
Tracking Status
firefox49 --- fixed

People

(Reporter: Gijs, Assigned: Gijs)

References

Details

Attachments

(1 file)

(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.
>+            let where = win.whereToOpenLink(data);
>+            Cu.reportError(where);
>+            if (where == "current") {
>+              where = "tab";
>+            }

"Cu.reportError(where)" ?????
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 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.
Attachment #8754774 - Flags: review?(jaws) → review+
https://hg.mozilla.org/mozilla-central/rev/c2a4ec679a40
Status: ASSIGNED → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 49
You need to log in before you can comment on or make changes to this bug.