1137234 - Middle click on search suggestion from about:home/newtab pages opens the searches in foreground tabs

Status: VERIFIED FIXED

(Firefox :: Search, defect)

Description

[Petruta Horea [:phorea], Desktop QA]

Reproducible on:
latest Developer Edition 38.0a2 
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:38.0) Gecko/20100101 Firefox/38.0
latest Nightly 39.0a1 - 20150225030226
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:39.0) Gecko/20100101 Firefox/39.0

Steps to reproduce:
1. Enter a string on about:home or about:newtab page 
2. Move the mouse on suggestions drop-down list
3. Click on them using the mouse middle click button

Actual results:
Middle click opens a foreground tab with the search results.

Expected results:
Searches should open in background tabs.

Notes: The issue reproduces also under Mac OSX 10.10 and Ubuntu 12.10 32-bit.

Comment 1

[Wickie Lee [:Robocrypt]]

Hi, I would like to work on this bug. I am new to contributing to open source. So if this is a bug that I could get some guidance on then it would be appreciated!

Comment 2

[Drew Willcoxon :adw]

Hi Wickie,

Thanks for volunteering!  This should be a simple bug to fix.  But first you need to set up a Firefox build environment.  If you haven't done that yet, please see these links:

https://developer.mozilla.org/en-US/docs/Introduction
https://developer.mozilla.org/en-US/docs/Mozilla/Developer_guide

There are two parts to this bug, about:home and about:newtab.

For about:home, you need to change "tab" on this line to "tabshifted": http://mxr.mozilla.org/mozilla-central/source/browser/modules/AboutHome.jsm?rev=2e9ff15fa9c7#201

For about:newtab, you need to remove these three lines: http://mxr.mozilla.org/mozilla-central/source/browser/modules/ContentSearch.jsm?rev=2e9ff15fa9c7#229  As you can see, if `data` says to use a new tab, then we set the selected tab to the new tab.  We should just not do that anymore.

And that should be it!  Once you've made your changes and you've tested them, you can attach your patch to this bug as described here: https://developer.mozilla.org/en-US/docs/Mozilla/Developer_guide/How_to_Submit_a_Patch#Submitting_the_patch

If you need help generating a patch in the first place, check here: https://developer.mozilla.org/en-US/docs/Mercurial_FAQ#How_can_I_generate_a_patch_for_somebody_else_to_check-in_for_me.3F

Good luck, let me know here in the bug when you have questions.

Comment 3

[Wilhem Barbier [:nounoursheureux]]

Wickie Lee: do you still want to take care of this bug ?

Comment 4

[njs1427]

:adw Hey, I would love to work on this as my first bug. I already have my enviroment setup and everything. Is it okay if I take this one?

Comment 5

[Drew Willcoxon :adw]

Sure, it doesn't look like Wickie is working on it.

Comment 6

[njs1427]

:adw Where could I find the files for about:home, and about:newtab?

Comment 7

[Drew Willcoxon :adw]

Please see comment 2.

Comment 8

[Keita Mimura]

Hello, I am new here and I worked on this and could I upload the patch file here if you don't mind?

Comment 9

[Keita Mimura]

Attachment: ForAboutHome(1137234).diff

there are two patches and this is the first one.

Comment 10

[Keita Mimura]

Attachment: ForContentSearch(1137234).diff

this is the second one.

Comment 11

[:Gavin Sharp [email: [e-mail]]]

Comment on attachment 8585771 [details] [diff] [review]
ForAboutHome(1137234).diff

Hey Keita - thanks for digging into this!

I'll flag Drew for feedback on these. It's typically a good idea to do that so that your patches don't get lost! https://developer.mozilla.org/en-US/docs/Mozilla/Developer_guide/How_to_Submit_a_Patch has a bit more background on the overall process.

Another thing to consider is that these two patches would ideally be combined. https://developer.mozilla.org/en-US/docs/Mercurial_FAQ#How_can_I_generate_a_patch_for_somebody_else_to_check-in_for_me.3F has a bit of background about how to generate patches using Mercurial, which would take care of that for you.

Comment 12

[Keita Mimura]

(In reply to :Gavin Sharp [email: [e-mail]] from comment #11)
> Comment on attachment 8585771 [details] [diff] [review]
> ForAboutHome(1137234).diff
> 
> Hey Keita - thanks for digging into this!
> 
> I'll flag Drew for feedback on these. It's typically a good idea to do that
> so that your patches don't get lost!
> https://developer.mozilla.org/en-US/docs/Mozilla/Developer_guide/
> How_to_Submit_a_Patch has a bit more background on the overall process.
> 
> Another thing to consider is that these two patches would ideally be
> combined.
> https://developer.mozilla.org/en-US/docs/
> Mercurial_FAQ#How_can_I_generate_a_patch_for_somebody_else_to_check-
> in_for_me.3F has a bit of background about how to generate patches using
> Mercurial, which would take care of that for you.

Thanks for replying! sorry I just started here.. I will try to use!

Comment 13

[Drew Willcoxon :adw]

Comment on attachment 8585771 [details] [diff] [review]
ForAboutHome(1137234).diff

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

Thanks, Keita, these both look good, so I'll land them now.

Comment 14

[Drew Willcoxon :adw]

Attachment: landed patch

Here's the single combined patch that I landed.

https://hg.mozilla.org/integration/fx-team/rev/af2151f856fb

Keita, I pushed your patch to our fx-team tree, which is where front-end Firefox development happens.  In the next day or so, someone will come along and merge your patch with the main Mozilla development tree, mozilla-central, and mark this bug as fixed.  Firefox Nightly is built from mozilla-central, so once that happens, you should be able to see your fix in the following Nightly.  Thanks!

Comment 15

[Ryan VanderMeulen [:RyanVM]]

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

Comment 16

[mergilmartin]

Verified fix (Firefox Nightly 40.0a1(2015-04-20) windows. 

I checked from about:home and about:newtab page

1. Enter a string on about:home or about:newtab page 
2. Move the mouse on suggestions drop-down list
3. Click on them using the mouse middle click button

Searches opened as background tab as expected.

Comment 17

[Petruta Horea [:phorea], Desktop QA]

Thanks for verifying, Martin.

I'm updating the status flags.