leave location bar suggestions open when I middle click or ctrl click an item
Categories
(Firefox :: Address Bar, enhancement, P3)
Tracking
()
People
(Reporter: markus, Unassigned)
References
Details
(Keywords: polish, ux-natural-mapping, Whiteboard: [qx:spec][fxsearch])
Attachments
(1 file, 1 obsolete file)
Updated•8 years ago
|
Comment 1•7 years ago
|
||
Comment 3•6 years ago
|
||
Comment 4•6 years ago
|
||
Updated•6 years ago
|
Comment 5•6 years ago
|
||
Comment 6•6 years ago
|
||
(In reply to Marco Bonardo [::mak] from comment #4)
I don't think we should implement this in the existing code, we should
rather try to implement this for the Quantum Bar code that we aim to enable
this year...
Due to Bug 1519589 this cannot be fully implemented yet. Would you prefer a patch for a partial implementation (Ctrl-click) soonish, or would it be better to wait for the quantumbar code to be more complete?
Comment 7•6 years ago
|
||
Nevermind, looks like Bug 1519589 is being fixed much quicker than I expected.
Comment 8•6 years ago
|
||
Not sure why some sections of code in the file uses
this.closePopup();
while others directly use
this.view.close();
so I just went with whatever was already there.
Found that clicking e.g. the 'home button' while the dropdown is open does not close it (it previously did on dev edition 65.0b12), but that also occurs without this patch.
Comment 10•6 years ago
|
||
(In reply to custom.firefox.lady [:tawn] from comment #8)
Not sure why some sections of code in the file uses
this.closePopup();
while others directly use
this.view.close();
so I just went with whatever was already there.
closePopup is a compatibility API for the old nsIAutoCompletePopup interface, it's just for plugging the new code... internally UrlbarInput should use this.view.close() imo, and closePopup should clarify that in its javadoc (we should also have a bug about changing it once we can remove the old code).
Comment 11•6 years ago
|
||
Comment 12•6 years ago
|
||
Just to note, this should also have the tests extended/fixed/new tests.
Comment 13•6 years ago
|
||
Has canonize been taken into consideration in the decision to implement this? Ctrl-Enter will react differently on the first item in the result list than the remaining items. (Users triggering canonize will be sent somewhere other than the displayed result.) Might it be better to implement this as Alt-click/Enter (Alt-Enter already defaults to opening url in new tab & it would eliminate code to check for macosx) or restrict this bug to mouse-only?
(In reply to Marco Bonardo [::mak] from comment #11)
::: browser/components/urlbar/UrlbarInput.jsm
@@ +292,5 @@
> pickResult(event, result) {
> this.setValueFromResult(result);
>
> + let modifKey = AppConstants.platform === "macosx" ? event.metaKey
> + : event.ctrlKey;
The code changed quite a bit, couldn't you just use "where" now?
We could use where == "tabshifted", but that would also keep the view open for Alt+Shift+Enter (though it should be perfect if we'd go with Alt-click/Enter instead of Ctrl-click/Enter).
I suspect in the tab switch case we should not keep the view open ...
Unless it's being overridden.
Let's do that for now then; ctrl/alt/shift already override & whether middle-click should be added to _toggleActionOverride can be decided later.
@@ -760,5 @@
> > - // Focus the content area before triggering loads, since if the load
> > - // occurs in a new tab, we want focus to be restored to the content
> > - // area when the current tab is re-selected.
> > - browser.focus();
shouldn't we still focus() here in the default case?
I was actually on the fence about this. There isn't a need to wait until popuphidden to change focus in the default case. OTOH, since the default case closes the view, doing them all on popuphidden makes the code a bit simpler. If you're aware of some reason the focus change in the default case should occur before the view closes, or just prefer it that way, fine; I don't feel strongly either way.
Comment 14•6 years ago
|
||
(In reply to custom.firefox.lady [:tawn] from comment #13)
Has canonize been taken into consideration in the decision to implement this? Ctrl-Enter will react differently on the first item in the result list than the remaining items. (Users triggering canonize will be sent somewhere other than the displayed result.) Might it be better to implement this as Alt-click/Enter (Alt-Enter already defaults to opening url in new tab & it would eliminate code to check for macosx) or restrict this bug to mouse-only?
That's a good question, this bug is a lot more complex than expected. The resulting matrix of possible outcomes becomes quite large, and I'm not so sure it's still manageable. I'm wondering if we shouldn't re-evaluate the actual need and request, if this is something many users want, or just a niche feature that we may decide to leave behind for too much complexity.
We don't even have a document for the interaction matrix, I can imagine it would be a nightmare for QA.
The bookmarks menus case was much simpler because there were less combinations.
Can we put this on the backburner for now? I feel like we should re-evaluate the whole interaction matrix first, we have too many keys doing the same thing and interacting each other, with canonization, switch-tab override, open in new tab, open in new background tab and open in new window all on shift/ctrl/alt modifiers and their combination.
At this point I wonder if the view should have a "sticky" feature instead of relying just on key combo.
Your suggestion to only do it for the mouse may also be ok, but we'd be left forever in an inconsistent state.
Comment 15•6 years ago
|
||
(In reply to Marco Bonardo [::mak] from comment #14)
We don't even have a document for the interaction matrix, I can imagine it would be a nightmare for QA.
...
Can we put this on the backburner for now? I feel like we should re-evaluate the whole interaction matrix first, we have too many keys doing the same thing and interacting each other, with canonization, switch-tab override, open in new tab, open in new background tab and open in new window all on shift/ctrl/alt modifiers and their combination.
After giving this further thought, IMO it would be much simpler to implement "Leave the menu/view open when opening a new background tab if openInTabClosesMenu pref set to false" (as opposed to current bug title). So we can just do if (!openInTabClosesMenu && where == "tabshifted")
eliminating need for OS checking and simply relying on preexisting 'where' code. Then nothing in that matrix changes except whether the view stays open.
Don't know if you or UX would be ok with that; we can postpone if you want.
Comment 16•5 years ago
|
||
From websites people are used to have tabs be opened in the background with their current context stay the same, if they open a link with a middle-click or a Ctrl/Cmd+click. This same behavior is now being implemented for bookmark menus and should also be the default behavior in the location bar.
The same behaviour should be true for the back / forward button & the reload button and the right-click menu of those buttons. Sometimes you want to open several tabs in the background without having to reopen the menu over and over again. The home button already has the behaviour.
Comment 17•5 years ago
|
||
Chrome has this feature on middle click and alt+shift+enter, so perhaps a parity-chrome flag should be added.
Also, as for the discussion on canonization: There's already a pref browser.urlbar.ctrlCanonizesURLs
, why not just follow that?
Updated•2 years ago
|
Description
•