1417527 - Right clicking on certain hyperlinks is wrongly considered as a left click

Status: VERIFIED FIXED

(Firefox :: Settings UI, defect, P4)

Description

[Marius Coman [:mcoman], Ecosystem QA]

Attachment: rec of the issue.gif

[Affected versions]:
- Firefox 57 and up

[Affected Platforms]:
- All Windows
- All Mac
- All Linux

[Steps to reproduce]:
1. Open the browser and navigate to "about:preferences#privacy" page.
2. Right click on the " clear your recent history" hyperlink and observe the behavior.

[Expected result]:
- A context menu is opened.

[Actual result]:
- The "Clear recent history" dialog box is opened.

[Notes]:
- The issue is reproducible if you right click the "remove individual cookies" or the "Change preferences for search engine suggestions" hyperlinks from the "about:preferences#privacy" page.
- The issue is also reproducible if you right click the "Go Back" hyperlink from the "about:preferences#containers" page.
- Attached a screen recording of the issue.

Comment 1

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

These are <label> elements with "click" event listeners. The event listeners should check the `event` argument to see which `event.button` was used to click on them.

See https://searchfox.org/mozilla-central/rev/550148ab69b2879bfb82ffa698720ede1fa626f2/browser/components/preferences/in-content/privacy.js#117-128 for three places, there may be more within this folder.

Comment 2

[Oliver Alonzo]

I'd be happy to take this one! It is my first bug :D

Comment 3

[Constantin Pavelescu]

Hello! I am going to try working on this one. This is also my first bug. 

I am trying to fix this as part of the coursework for the 389COM Open Source Development module at Coventry University. 

I will come back here and post comments if I need any help throughout the process.

Thank you, 
Constantin Sorin Pavelescu 
Coventry University

Comment 4

[Oliver Alonzo]

Attachment: Remove right click behavior as left click by checking the event.button

Comment 5

[Oliver Alonzo]

Attachment: Bug 1417527 - Remove right click behavior as left click by checking the event.button.

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

Comment 6

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

Comment on attachment 8930199 [details] [diff] [review]
Remove right click behavior as left click by checking the event.button

Review of attachment 8930199 [details] [diff] [review]:
-----------------------------------------------------------------

Thanks for the patch, this looks good. Can you please make the following change to all three files? After you make that change and reupload the patch it should be ready.

::: browser/components/preferences/in-content/containers.js
@@ +18,5 @@
>      this._list = document.getElementById("containersView");
>  
> +    document.getElementById("backContainersLink").addEventListener("click", function(event) {
> +      if (event.button == 0) {
> +          gotoPref("general");

nit, please use two space indentation to be consistent with the surrounding code.

Comment 7

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

Comment on attachment 8930205 [details]
Bug 1417527 - Remove right click behavior as left click by checking the event.button.

https://reviewboard.mozilla.org/r/201356/#review206718

Same comment on this version as the previous one. I do prefer patches uploaded through mozreview so thank you for uploading this version :)

Comment 8

[Oliver Alonzo]

Attachment: Bug 1417527 - Fix indentation to be consistent with standards.

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

Comment 9

[Oliver Alonzo]

I have made the changes and submitted for review with mozreview:

https://reviewboard.mozilla.org/r/202142/

Thank you for pointing out the difference. I didn't realize I submitted it twice last time!

Comment 10

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

Comment on attachment 8931045 [details]
Bug 1417527 - Fix indentation to be consistent with standards.

https://reviewboard.mozilla.org/r/202142/#review207610

Comment 11

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

Comment on attachment 8930205 [details]
Bug 1417527 - Remove right click behavior as left click by checking the event.button.

https://reviewboard.mozilla.org/r/201356/#review207612

Comment 12

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/70140253fc37
Remove right click behavior as left click by checking the event.button. r=jaws

Comment 13

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

Congratulations on your first patch! I just pushed it to our inbound repository and it should show up in Firefox Nightly builds within a day or two.

I had to adjust your patch a little because of two issues that I failed to notice when I was reviewing it.

1. By adding the `if` and moving the `return false` inside of it, it meant that not all code paths returned the same type from the function. If event.button was 0, then we returned false. Otherwise we returned undefined. I moved the `return false` outside of the if-branch to fix this.

2. We use double-quotes for all of our strings and the patch added a single-quoted string. I changed this to use double-quotes.

Both of these were noticed by our linter tool, which you can learn more about at https://firefox-source-docs.mozilla.org/tools/lint/usage.html

Comment 14

[Oliver Alonzo]

Perfect. Thanks!!

Comment 15

[Cosmin Sabou [:CosminS]]

https://hg.mozilla.org/mozilla-central/rev/70140253fc37

Comment 16

[Tanvir Rahman]

I have reproduced this bug with Nightly 59.0a1 (2017-11-15) on ubuntu 16.04

The bug's fix is now verified with Latest Beta 59.0b10

Build ID 20171115100050
User Agent Mozilla/5.0 (X11; Linux x86_64; rv:59.0) Gecko/20100101 Firefox/59.0

Comment 17

[Tanvir Rahman]

Little typo mistake. This is the build where i found the fix (Beta 59.0b10)
Build ID 	20180215111455
User Agent 	Mozilla/5.0 (X11; Linux x86_64; rv:59.0) Gecko/20100101 Firefox/59.0

Comment 18

[Mohammad Maruf Rahman]

I have reproduced this bug with Nightly 59.0a1 (2017-11-15) on Windows 10, 64 Bit!

The bug's fix is now verified with Latest Beta!

Build ID   20180215111455
User Agent Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:59.0) Gecko/20100101 Firefox/59.0

[testday-20180216]

Comment 19

[Tanvir Rahman]

As per comment 16,17,18 i am marking this bug as "Verified Fixed"

Comment 20

[Oana Botisan, Desktop QA]

I verified the fix on latest Nightly 60.0a1 and beta 59.0b10 on Windows 10 x64, macOS 10.13 and Ubuntu 16.04 x64 and the bug is partially not reproducing. When you right-click on "clear your recent history" the "Clear Recent History" is not displayed anymore, but neither is the context menu. 

Is that the expected behaviour? Does the context menu shouldn't be displayed when you right-click on the hyperlink?