Make link modifiers work consistently throughout Firefox

RESOLVED FIXED in Firefox1.0beta

Status

()

--
enhancement
RESOLVED FIXED
15 years ago
7 years ago

People

(Reporter: jruderman, Assigned: jruderman)

Tracking

(Blocks: 1 bug)

unspecified
Firefox1.0beta
x86
Windows XP
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: fixed-aviary1.0)

Attachments

(1 attachment, 2 obsolete attachments)

(Assignee)

Description

15 years ago
I'm working on a patch that will make middle-click, alt, shift, ctrl, and
ctrl+shift work on link-like controls throughout Firefox's UI.  Right now, each
of those modifiers works in some places but not in others.  My patch will also
make all link-like controls focus the content area as they load a URL.
(Assignee)

Updated

15 years ago
Depends on: 246720

Comment 1

15 years ago
Exactly what will change with this patch? What modifiers are used and where?
(Assignee)

Comment 2

15 years ago
Ctrl        new tab, selected
Shift       new window
Ctrl+Shift  new tab, in background
Alt         save

You can swap Ctrl and Ctrl+shift by toggling the hidden pref
browser.tabs.loadBookmarksInBackground (not browser.tabs.loadInBackground).

Middle-clicking is the same as Ctrl+clicking (it opens a new tab) and it is
subject to the shift modifier and pref in the same way.

When I'm done, hopefully the modifiers and middle-clicking will work in the
following places, with a few exceptions:

Moz#   Fx#

72361  236864 bookmarks menu (ctrl+shift opened a new window)
89922         bookmarks in sidebar (ctrl+shift opened a new window)
              history sidebar (ctrl+shift opened a new window)
93034         throbber
       214816 go menu: sites
       224132 "view image", "view background image" context menu items
              help > release notes
              middle-click paste (Linux)
              items in the back and forward menus
75731  202819 back, forward buttons (without copying history)

              go menu: back, forward
164008        address bar history
              address bar autocomplete
       198028 go button
              dragging to address bar
233411        dragging to search bar
              "Search Web for ..." context menu item
              enter in search bar (parity-GoogleToolbarForIE)
              home button (at least when there's only one home page)

So far, I have the first ten working (through the break in the list), but with
some bugs: middle-clicking some things doesn't work unless you left-click them
first (due to bug 246720), and the back and forward buttons always appear enabled.

I'm still thinking about whether to do the reload button (Ctrl+reload would open
the current page in a new tab).  Shift+reload has a special meaning dating back
to Netscape 4, which I don't want to change.

Comment 3

15 years ago
(In reply to comment #2)
> Ctrl        new tab, selected
> Shift       new window
> Ctrl+Shift  new tab, in background
> Alt         save

The default for Firefox is to load tabs in the background, so Ctrl should not
spawn a new selected tab, and Ctrl+Shift should not spawn a background tab; it
should be the other way around. Maybe that's what you meant?
(Assignee)

Comment 4

15 years ago
I'm going for consistency with bookmarks (bug 236864).  I does make sense in a
way: the main reason anyone wants tab to load in the background is so opening
links from web pages doesn't interfere with reading or clicking more links. 
When you're opening links from other places, that reason doesn't make sense.

Anyway, once my patch is in, it will be easier to change than it is now :)

Comment 5

15 years ago
How about form submission buttons? There is, as far as I know, no way to see the
results of submitting a form in a new tab while keeping the form visible.
(Assignee)

Comment 6

15 years ago
I'm not going to fix form submission buttons because that would require backend
work.  See bug 17754.

Comment 7

15 years ago
(In reply to comment #2)
>               home button (at least when there's only one home page)
Bug 232172 already covers that. My patch for that also works with multiple home
pages. It doesn't respect the pref browser.tabs.loadBookmarksInBackground yet
though.
(Assignee)

Comment 8

15 years ago
Created attachment 150991 [details] [diff] [review]
patch

This patch:
* Makes middle-click, shift+click, ctrl+click, and ctrl+shift+click work
consistently in many places in Firefox's UI.
* Puts focus in the content area after you left-click Go menu items, etc.
* Makes all UI links follow the hidden pref "open bookmarks in background".
* Makes mailto: bookmarks work.  (Before, they said "mailto: is not a
registered protocol.  I don't know why.)

It makes the modifiers work in:
72361  236864 bookmarks menu (ctrl+shift opened a new window)
89922	      bookmarks in sidebar (ctrl+shift opened a new window)
	      history sidebar (ctrl+shift opened a new window)
93034	      throbber
       214816 go menu: sites
       224132 "view image", "view background image" context menu items
	      help > release notes
	      middle-click paste (Linux)
	      items in the back and forward menus
75731  202819 back, forward buttons (without copying history)
	      go menu: back, forward

This patch needs to be tested on Mac and Linux to make sure I got the modifiers
right on those platforms.  On Linux, the "middle-click paste to load URL"
feature needs to be tested with various things in the clipboard and with
various modifiers.
(Assignee)

Updated

15 years ago
No longer blocks: 198028
No longer depends on: 246720

Updated

15 years ago
Blocks: 232172
(Assignee)

Comment 9

15 years ago
Created attachment 151042 [details] [diff] [review]
better patch

Improvements from previous patch:
* Now includes a port of the patch from bug 217090, since I needed that to test
middlemouse.contentLoadURL on Windows.	This change only affects non-Linux
users who have middlemouse.contentLoadURL set to true.
* No longer breaks middle click paste to load URLs (typo fix).
* Slightly better comments.
(Assignee)

Updated

15 years ago
Attachment #150991 - Attachment is obsolete: true
(Assignee)

Updated

15 years ago
Attachment #151042 - Flags: review?(bryner)

Comment 10

15 years ago
I guess an "onmiddleclick" event handler would be handy here.
Comment on attachment 151042 [details] [diff] [review]
better patch

>--- base/content/browser-context.inc	10 Apr 2004 05:04:09 -0000	1.4
>+++ base/content/browser-context.inc	17 Jun 2004 19:26:05 -0000
>@@ -132,7 +133,8 @@
>       <menuitem id="context-viewbgimage"
>                 label="&viewBGImageCmd.label;"
>                 accesskey="&viewBGImageCmd.accesskey;"
>-                oncommand="gContextMenu.viewBGImage();"/>
>+                oncommand="gContextMenu.viewBGImage(event);"
>+                onclick="if(event.button==1) { eval(this.getAttribute('oncommand')); event.target.parentNode.hidePopup(); }"/>
>       <menuitem id="context-undo"
>                 label="&undoCmd.label;"
>                 accesskey="&undoCmd.accesskey;"

>--- base/content/browser-menubar.inc	7 Jun 2004 08:16:53 -0000	1.24
>+++ base/content/browser-menubar.inc	17 Jun 2004 19:26:05 -0000
>@@ -268,10 +268,30 @@
>               </menupopup>
>             </menu>
>   
>-            <menu label="&goMenu.label;" accesskey="&goMenu.accesskey;" oncommand="var url = event.target.getAttribute('statustext'); if (url) openTopWin(url);">
>+            <menu label="&goMenu.label;" accesskey="&goMenu.accesskey;" 
>+                  oncommand="var url = event.target.getAttribute('statustext'); if (url) openUILink(url, event, false, true);"
>+                  onclick="if(event.button==1) { event.target.parentNode.hidePopup(); eval(this.getAttribute('oncommand')); }">
>+

Just for sanity, try to be consistent about the order of running oncommand and
hiding the popup, unless there's a reason for the difference.  Maybe this
should all be factored out into a function, since it's repeated so much?

Looks good to me other than that.
Attachment #151042 - Flags: review?(bryner) → review+
(Assignee)

Comment 12

15 years ago
Created attachment 151194 [details] [diff] [review]
patch addressing bryner's review comments

I added a function "checkForMiddleClick" to utilityOverlay.js.
Attachment #151042 - Attachment is obsolete: true
(Assignee)

Updated

15 years ago
Attachment #151194 - Flags: review?(bryner)
(Assignee)

Comment 13

15 years ago
aebrahim tested the patch on Linux for me.  Now I know that it works correctly
on Windows and Linux :)
Comment on attachment 151194 [details] [diff] [review]
patch addressing bryner's review comments

Looks good.
Attachment #151194 - Flags: review?(bryner) → review+
(Assignee)

Comment 15

15 years ago
Fix checked in 2004-06-23 15:54 PST.
Status: NEW → RESOLVED
Last Resolved: 15 years ago
Resolution: --- → FIXED
(Assignee)

Comment 16

15 years ago
Patch checked in on AVIARY_1_0_20040515_BRANCH too.  I had to apply the last
hunk (removing the old version of OpenURL in history.js) manually; I don't know why.
Whiteboard: fixed-aviary1.0

Comment 17

15 years ago
How about modifiers on Javascript links and such? Is it possible to detect
link.open() in Javascript and apply the link modifier in such case? If it works,
then pages such as Hotmail will work flawlessly.
(Assignee)

Comment 18

15 years ago
Cheng: that's bug 55696 or bug 138198.

Comment 19

15 years ago
*** Bug 250086 has been marked as a duplicate of this bug. ***
(Assignee)

Updated

15 years ago
Target Milestone: --- → Firefox1.0beta

Comment 20

15 years ago
*** Bug 251189 has been marked as a duplicate of this bug. ***
(Assignee)

Comment 21

14 years ago
*** Bug 255299 has been marked as a duplicate of this bug. ***

Comment 22

14 years ago
What about URL bar dropdown, Go button, URL bar ctrl+enter and search bar
ctrl+click (bug 198028, bug 64008, bug 162119 and ?)? Also, middle-click on
bookmark doesn't respect open-in-the-background pref.

Comment 23

14 years ago
Did, at any point, a checkin from the bug make ctrl-click/middle-click on reload
open it in a new tab? This was working for me at one point, I just don't recall
if it was thanks to an extension. If it was innate to Firefox, it has recently
regressed. Please bring it back! No use in having a middle click on that button
do absolutely nothing, and I found reloading the current page in a new tab often
very useful. ("Compare this old version of this page to the current version"
great for news articles, web development, network monitors... anything that
changes).
(Assignee)

Comment 24

14 years ago
> What about URL bar dropdown, Go button, URL bar ctrl+enter and search bar
> ctrl+click (bug 198028, bug 64008, bug 162119 and ?)?

I skipped things related to the address and search field because they're weird
(alt instead of ctrl for new tab).  File a bug :)

> Also, middle-click on bookmark doesn't respect open-in-the-background pref.

The pref for bookmarks is separate from the pref for links.  This will be more
clear in Firefox 1.0 Preview Release, and you'll be able to toggle both prefs.

> Did, at any point, a checkin from the bug make ctrl-click/middle-click on 
> reload open it in a new tab?

No.  That would be trickier to implement because it would require getting a new
version of the page, possibly copying POSTDATA, etc.  File a bug :)

Comment 25

14 years ago
I know this bug is closed, but why not swap the SHIFT and ALT behaviours?
SHIFT-click for saving has been the behaviour for ages. Changing this for ALT
can be counter intuitive for many users.
(Assignee)

Comment 26

14 years ago
*** Bug 261164 has been marked as a duplicate of this bug. ***
(Assignee)

Comment 27

14 years ago
See also: bug 263942, bug 279687.

Comment 28

12 years ago
Comment on attachment 151194 [details] [diff] [review]
patch addressing bryner's review comments

>+function whereToOpenLink( e, ignoreButton, ignoreAlt )
>+{
>+  if (e == null)
>+    e = { shiftKey:false, ctrlKey:false, metaKey:false, altKey:false, button:0 };
>+
>+  var shift = e.shiftKey;
>+  var ctrl =  e.ctrlKey;
>+  var meta =  e.metaKey;
>+  var alt  =  e.altKey && !ignoreAlt;
>+
>+  // ignoreButton allows "middle-click paste" to use function without always opening in a new window.
>+  var middle = !ignoreButton && e.button == 1;
>+  var middleUsesTabs = getBoolPref("browser.tabs.opentabfor.middleclick", true);

>-  getBrowserTargetFromEvent: function (aEvent)
>-  {
>-    var button = (aEvent.type == "command" || aEvent.type == "keypress") ? 0 :aEvent.button;

getBrowserTargetFromEvent used to check that the event has a button. whereToOpenLink does not...
(Assignee)

Comment 29

12 years ago
Does that cause problems, such as incorrect behavior or javascript strict warnings?
(Assignee)

Updated

11 years ago
Depends on: 422732
You need to log in before you can comment on or make changes to this bug.