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

VERIFIED FIXED in Firefox 59



2 years ago
a year ago


(Reporter: mcoman, Assigned: oliveralonzof, Mentored, NeedInfo)



Firefox 59

Firefox Tracking Flags

(firefox57 wontfix, firefox58 wontfix, firefox59 fixed)



(3 attachments, 1 obsolete attachment)

Posted image 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.

- 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.
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.
Mentor: jaws
Keywords: good-first-bug
Priority: -- → P4

Comment 2

2 years ago
I'd be happy to take this one! It is my first bug :D
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 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.
Attachment #8930199 - Flags: review-
Assignee: nobody → oliveralonzof
Comment on attachment 8930205 [details]
Bug 1417527 - Remove right click behavior as left click by checking the event.button.


Same comment on this version as the previous one. I do prefer patches uploaded through mozreview so thank you for uploading this version :)
Attachment #8930205 - Flags: review?(jaws) → review-
Comment hidden (mozreview-request)

Comment 9

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


Thank you for pointing out the difference. I didn't realize I submitted it twice last time!
Comment on attachment 8931045 [details]
Bug 1417527 - Fix indentation to be consistent with standards.

Attachment #8931045 - Flags: review?(jaws) → review+
Comment on attachment 8930205 [details]
Bug 1417527 - Remove right click behavior as left click by checking the event.button.

Attachment #8930205 - Flags: review- → review+

Comment 12

2 years ago
Pushed by jwein@mozilla.com:
Remove right click behavior as left click by checking the event.button. r=jaws
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

2 years ago
Perfect. Thanks!!

Comment 15

2 years ago
Last Resolved: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 59
QA Whiteboard: [good first verify]

Comment 16

a year ago
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
QA Whiteboard: [good first verify] → [good first verify] [testday-20180216]

Comment 17

a year ago
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
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


Comment 19

a year ago
As per comment 16,17,18 i am marking this bug as "Verified Fixed"
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?
Flags: needinfo?(oliveralonzof)
You need to log in before you can comment on or make changes to this bug.