As a security precaution, we have turned on the setting "Require API key authentication for API requests" for everyone. If this has broken something, please contact bugzilla-admin@mozilla.org
Last Comment Bug 1120967 - Broken middle/right click on links via about:preferences pages
: Broken middle/right click on links via about:preferences pages
Status: VERIFIED FIXED
:
Product: Firefox
Classification: Client Software
Component: Preferences (show other bugs)
: Trunk
: All All
: -- normal (vote)
: Firefox 51
Assigned To: Fischer [:Fischer]
:
: Jared Wein [:jaws] (please needinfo? me)
Mentors:
: 1273515 1297170 (view as bug list)
Depends on: 1301813
Blocks: 718011 1028029
  Show dependency treegraph
 
Reported: 2015-01-13 06:57 PST by Ada [:adalucinet]
Modified: 2017-01-06 12:40 PST (History)
10 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard: [testday-20160909]
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---
verified

MozReview Requests
Submitter Diff Changes Open Issues Last Updated
Loading...
Error loading review requests:
Show discarded requests

Attachments
popup window.png (127.91 KB, image/png)
2016-07-27 23:43 PDT, Jack Lin[:jacklin]
no flags Details
Bug 1120967 - Broken middle/right click on links via about:preferences pages; (58 bytes, text/x-review-board-request)
2016-08-15 01:57 PDT, Fischer [:Fischer]
jaws: review+
Details | Review
html_a_help_button_UI.png (678.82 KB, image/png)
2016-09-04 20:27 PDT, Fischer [:Fischer]
no flags Details

Description User image Ada [:adalucinet] 2015-01-13 06:57:26 PST
Reproducible on 36 beta 1 (Build ID: 20150112201205), Aurora 37.0a2 (Build ID: 20150113004007) and Nightly 38.0a1 (Build ID: 20150113030205)
Platforms: Windows 7 32-bit, Windows 8.1 64-bit, Mac OSX 10.9.5 and Ubuntu 14.04 32-bit

Steps to reproduce:
1. Launch Firefox.
2. Go to about:preferences
3. Middle/right click on links via available pages - Eg:
-- Go to about:preferences#privacy and right/middle click on 'Learn more', 'clear your recent history' and 'remove individual cookies'.

Expected result: Links are opened in new tabs (same behavior as in about:preferences#sync).
Actual result: Nothing happens and 'uncaught exception: Load of # denied.' message is displayed in Browser console when middle click on 'clear your recent history' and 'remove individual cookies'.

Notes:
1. Not a regression - reproducible with Nightly 2014-05-19.
2. In about:preferences#privacy, a context menu is displayed for 'clear your recent history' and 'remove individual cookies'; when 'Bookmark this link' is selected: 'NS_ERROR_MALFORMED_URI: Component returned failure code: 0x804b000a (NS_ERROR_MALFORMED_URI) [nsIIOService2.newURI]' message is thrown by BrowserUtils.jsm:70:0 in Browse Console.
Comment 1 User image Fischer [:Fischer] 2016-07-20 23:54:53 PDT
@Jack & Tina,
I guess we need to think about one unified behavior of middle/right click on links.
Please have a look, thanks
Comment 2 User image Jack Lin[:jacklin] 2016-07-27 23:43:57 PDT
Created attachment 8775451 [details]
popup window.png

Hi Fischer,

We would like to make browser experience consistent, so

About right click,

1.For opening a website link, such as 
    'Learn more', 
    'Question mark', 
    'Search-Add more search engines...', 
    'Privacy-Change preferences for search engine suggestions...', 
    'Sync-Manage Account', 
    'Sync-Android/iOS', 
    'Sync-Terms of Service/Privacy Notice', 
can show a popup window like the attachment?

2.For launching an action, such as 'clear your recent history' and 'remove individual cookies', please keep current design.


About middle click,

Is it possible to make middle click same as left click? For example,

1. When the user presses middle click on a website link 'Learn more', open the website on a new tab. 

2. When the user presses middle click on a link 'clear your recent history', show 'Clear Recent History' popup window.

3. When the user presses middle click on a button 'Make Default', show its popup window.


Thank you
Comment 3 User image Fischer [:Fischer] 2016-08-15 01:57:56 PDT Comment hidden (mozreview-request)
Comment 4 User image Fischer [:Fischer] 2016-08-15 02:04:02 PDT Comment hidden (mozreview-request)
Comment 5 User image Fischer [:Fischer] 2016-08-16 05:47:15 PDT Comment hidden (mozreview-request)
Comment 6 User image Fischer [:Fischer] 2016-08-16 06:01:12 PDT
(In reply to Jack Lin[:jacklin] from comment #2)
> Created attachment 8775451 [details]
> popup window.png
> 
> Hi Fischer,
> 
> We would like to make browser experience consistent, so
> 
> About right click,
> 
> 1.For opening a website link, such as 
>     'Learn more', 
>     'Question mark', 
>     'Search-Add more search engines...', 
>     'Privacy-Change preferences for search engine suggestions...', 
>     'Sync-Manage Account', 
>     'Sync-Android/iOS', 
>     'Sync-Terms of Service/Privacy Notice', 
> can show a popup window like the attachment?
> 
> 2.For launching an action, such as 'clear your recent history' and 'remove
> individual cookies', please keep current design.
> 
> 
> About middle click,
> 
> Is it possible to make middle click same as left click? For example,
> 
> 1. When the user presses middle click on a website link 'Learn more', open
> the website on a new tab. 
> 
> 2. When the user presses middle click on a link 'clear your recent history',
> show 'Clear Recent History' popup window.
> 
> 3. When the user presses middle click on a button 'Make Default', show its
> popup window.
> 
> 
> Thank you


After discussion and demo with the UX :jalin, the unified middle/right mouse click behaviors are:

* For right-mouse clicking a website link, such as 
     'Learn more', 
     'Question mark'(The "?" button on the top-right side), 
     'Search-Add more search engines...', 
     'Sync-Manage Account',
     'Sync-Android/iOS', 
     'Sync-Terms of Service/Privacy Notice', 
will show a menu like the attachment of "popup window.png"

* For middle-mouse clicking a website link will open that link in a new tab

* For middle/right-mouse clicking
     'Privacy - Change preferences for search engine suggestions...', 
     'Privacy - clear your recent history',
     'Privacy - show 'Clear Recent History',
     'Privacy - Manage your Do Not Track settings"
will launch the corresponding function.
Comment 7 User image Fischer [:Fischer] 2016-08-16 06:16:38 PDT
Hi Jared,

This patch implements the unified middle/right mouse click behaviors based on Comment 6.
What it does is simply to put all website links inside html:a elements. As a result, the nsContextMenu at [1] would recognize and open the menu when right-mouse clicking.
In order to achieve this, this patch also modifies some utility JS functions and CSS rules.

As for the middle/right-mouse clicking parts, the patch basically do nothing about it since the default behavior of middle-mouse clicking on html:a would open that link and the default behavior on middle/right-mouse clicking on, like 'Privacy - clear your recent history', would trigger the corresponding function.

The try is at [2].

Thank you

[1] https://dxr.mozilla.org/mozilla-central/source/browser/base/content/nsContextMenu.js#614
[2] https://treeherder.mozilla.org/#/jobs?repo=try&revision=5d00977a868c
Comment 8 User image Fischer [:Fischer] 2016-08-16 06:19:21 PDT
Comment on attachment 8781045 [details]
Bug 1120967 - Broken middle/right click on links via about:preferences pages;

https://reviewboard.mozilla.org/r/71560/#review69414

::: browser/components/preferences/in-content/privacy.xul:284
(Diff revision 3)
>              preference="browser.urlbar.suggest.bookmark"/>
>    <checkbox id="openpageSuggestion" label="&locbar.openpage.label;"
>              accesskey="&locbar.openpage.accesskey;"
>              preference="browser.urlbar.suggest.openpage"/>
> -  <label class="text-link" onclick="if (event.button == 0) gotoPref('search')">
> +  <label class="text-link" onclick="gotoPref('search')">
>      &suggestionSettings.label;

Remove the condition of if (event.button == 0) so middle/right mouse click would also work.
Comment 9 User image Jared Wein [:jaws] (please needinfo? me) 2016-08-17 08:28:17 PDT
*** Bug 1273515 has been marked as a duplicate of this bug. ***
Comment 10 User image Jared Wein [:jaws] (please needinfo? me) 2016-08-17 08:31:17 PDT
Comment on attachment 8781045 [details]
Bug 1120967 - Broken middle/right click on links via about:preferences pages;

https://reviewboard.mozilla.org/r/71560/#review69906

Bug 1268943 should have made it so that you didn't need to make most of these changes. Anything that is a xul:label with class="text-link" should work.
Comment 11 User image Fischer [:Fischer] 2016-08-18 02:12:40 PDT
@Jared,

Bug 1268943 does not cover all of this bug.
Like Comment 6, this patch enables context menu on external links when right-mouse clicking. 

Because the namespace of <label class="text-link"> is XUL namespace, context menu is not allowed to be displayed on it when calling nsContextMenu#setTarget at [1].
This patch moves external links from <label class="text-link"> to <html:a> so as to enable context menu.

This change is quite reasonable because those external "HTTP" links should have context menu just like other links on web pages. Users would have consistent UX on links.

And thanks for the works in Bug 1268943. This patch basically doesn't have to handle middle-mouse click.

Maybe we should change this bug title to "No context menu when right clicking on external links inside about:preferences page".

How do you think about this UX improvement?

[1] https://dxr.mozilla.org/mozilla-central/source/browser/base/content/nsContextMenu.js#627
Comment 12 User image Jared Wein [:jaws] (please needinfo? me) 2016-08-18 10:23:48 PDT
We can relax the check in nsContextMenu since it was only added for right-clicking on buttons. If it's a label with class="text-link" we could let it through, though some code may have to be refactored. This would also fix the same issue with about:addons when you right click on the author's name of an addon. 

If we allow right-clicking we should also allow middle-clicking.

I don't see much gain in changing the help buttons to html:a, as they already open in a new tab. I'm not sure what other actions people would do with these buttons.
Comment 13 User image Jared Wein [:jaws] (please needinfo? me) 2016-08-18 14:30:29 PDT
I forgot to link it but it was bug 404536 which added the check for !XUL in nsContextMenu.js
Comment 14 User image Fischer [:Fischer] 2016-08-23 00:03:11 PDT Comment hidden (mozreview-request)
Comment 15 User image Fischer [:Fischer] 2016-08-23 00:22:00 PDT
Comment on attachment 8781045 [details]
Bug 1120967 - Broken middle/right click on links via about:preferences pages;

Change r? to f?
-------------------------------
Hi Jared,

Try the relaxing <label class="text-link"> approach. Please notice that even relaxing <label class="text-link">, still need to modify some JS utility methods in order to set href attribute.

> If we allow right-clicking we should also allow middle-clicking.
- Currently middle-clicking on <label class="text-link"> already opens new tab so what else should be take cared ?

> I don't see much gain in changing the help buttons to html:a, as they already open in a new tab.
- From the UX's point, the right-click context menu provides user more options, such as "Open Link in New Window", "Copy Link Location". These options are what user may want to act on a link so that would be good to have it.
To change that "?" link, there are 2 approaches. One is change to <label class="text-link">, another is <html:a>. Do we have any concern or I am thinking that using web standard <html:a> may be better.

Thank you
Comment 16 User image Jared Wein [:jaws] (please needinfo? me) 2016-08-25 13:09:45 PDT
*** Bug 1297170 has been marked as a duplicate of this bug. ***
Comment 17 User image Jared Wein [:jaws] (please needinfo? me) 2016-08-29 20:09:46 PDT
Comment on attachment 8781045 [details]
Bug 1120967 - Broken middle/right click on links via about:preferences pages;

https://reviewboard.mozilla.org/r/71560/#review72992

This looks good (feedback+ from me).

We should add a test case the changes to nsContextMenu.js.

I'm OK with changing the Help button to be an html:a link as long as we can keep the styling, keyboard access, and focus-ring behavior unchanged.

::: browser/base/content/nsContextMenu.js:821
(Diff revision 4)
> -             ((elem instanceof HTMLAnchorElement && elem.href) ||
> +             (isXULTextLinkLabel(elem) ||
> +              (elem instanceof HTMLAnchorElement && elem.href) ||

browser_contextMenu.js needs to have a test case added.
Comment 18 User image Fischer [:Fischer] 2016-09-04 20:27:25 PDT
Created attachment 8788028 [details]
html_a_help_button_UI.png
Comment 19 User image Fischer [:Fischer] 2016-09-04 20:27:54 PDT Comment hidden (mozreview-request)
Comment 20 User image Fischer [:Fischer] 2016-09-04 20:34:43 PDT
@Jared,

The help buttons are updated to <html:a class="help-button>. The UI styling is kept, please see the html_a_help_button_UI.png.

The test case for xul text-link label has been added.

The local test is passed and the try is here: https://treeherder.mozilla.org/#/jobs?repo=try&revision=a97085618b8c

Please have a look.

Thank you.
Comment 21 User image Jared Wein [:jaws] (please needinfo? me) 2016-09-06 12:51:22 PDT
Comment on attachment 8781045 [details]
Bug 1120967 - Broken middle/right click on links via about:preferences pages;

https://reviewboard.mozilla.org/r/71560/#review75208

::: browser/components/preferences/in-content/preferences.js:85
(Diff revision 5)
> +  for (let i of items) {
> +    // Extract category from id. For instance, extract "sync" from "category-sync".
> +    id = i.getAttribute("id");
> +    category = id.substr(id.indexOf("-") + 1);
> +    helpBtn = document.querySelector("#header-" + category + " > .help-button");
> +    helpBtn.setAttribute("href", getHelpLinkURL(i.getAttribute("helpTopic")));
> +  }

We already have code that will get the category name from the richlistitem's value. Also, please use `categories` instead of `items` and `category` instead of `i`. Also, please use template strings.

for (let category of categories) {
  let value = category.value;
  let name = internalPrefCategoryNameToFriendlyName(value);
  let helpSelector = `#header-${name} > .help-button`;
  let helpButton = document.querySelector(helpSelector);
  helpButton.setAttribute("href", getHelpLinkURL(category.getAttribute("helpTopic")));
}

::: toolkit/themes/shared/in-content/common.inc.css:316
(Diff revision 5)
> +
> +html|*.help-button:hover {
> +  background-image: url("chrome://global/skin/in-content/help-glyph.svg#help-hover");
> +}
> +
> +html|*.help-button:active {

html|*.help-button:hover:active {
  ...
}
Comment 22 User image Fischer [:Fischer] 2016-09-07 04:23:12 PDT Comment hidden (mozreview-request)
Comment 24 User image Pulsebot 2016-09-07 14:53:06 PDT
Pushed by ryanvm@gmail.com:
https://hg.mozilla.org/integration/fx-team/rev/7ec0c9ca41df
Broken middle/right click on links via about:preferences pages. r=jaws
Comment 25 User image Carsten Book [:Tomcat] 2016-09-08 03:08:11 PDT
https://hg.mozilla.org/mozilla-central/rev/7ec0c9ca41df
Comment 26 User image Saheda Reza [:Antora] 2016-09-09 16:04:09 PDT
I have reproduced this on firefox nightly according to(2015-01-13)

Fixing bug is verified on Latest Nightly-- Build ID:(20160908030434 ), User Agent: Mozilla/5.0 (Windows NT 10.0; rv:51.0) Gecko/20100101 Firefox/51.0

Those issues are solved.It seems ok on latest Nightly. :-)

Tested OS--Windows10 32bit
Comment 27 User image Maruf Rahman[:mMARUF] 2017-01-06 03:25:42 PST
I have reproduce this bug with Nightly 38.0a1 (2015-01-13) on Ubuntu 16.04 LTS !

This bug's fix is verified with latest Beta !

Build Id   :   20161219140923
User Agent :   Mozilla/5.0 (X11; Linux x86_64; rv:51.0) Gecko/20100101 Firefox/51.0
[testday-20170106]
Comment 28 User image Jared Wein [:jaws] (please needinfo? me) 2017-01-06 12:40:12 PST
Thanks!

Note You need to log in before you can comment on or make changes to this bug.