Textbox context menu generates spurious focus events.

RESOLVED FIXED

Status

()

Core
XUL
RESOLVED FIXED
16 years ago
16 years ago

People

(Reporter: neil@parkwaycc.co.uk, Assigned: jag (Peter Annema))

Tracking

({polish})

Trunk
polish
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 1 obsolete attachment)

8.63 KB, patch
jag (Peter Annema)
: review+
jag (Peter Annema)
: superreview+
Details | Diff | Splinter Review
(Reporter)

Description

16 years ago
As Dean Tessman noticed in his patch to bug 62495 and bug 96446, a context menu
click in a textbox generates extra focus events.

Unfortunately in fixing this I'm having to redo his URL Bar work :-(
(Reporter)

Comment 1

16 years ago
Created attachment 90282 [details] [diff] [review]
Proposed patch
(Reporter)

Comment 2

16 years ago
I think this maintains the same click-selects-all behaviour as before, i.e.
1. Tabbing to URL Bar selects all
2. Mouse down in unfocused URL Bar starts a new selection
3. Mouse click in unfocused unselected URL Bar will select all
Keywords: patch, polish, review

Comment 3

16 years ago
1.
-function URLBarMouseUpHandler(aEvent)
+function URLBarClickHandler(aEvent)
 {
-  if (gMouseDownX > -1 && Math.abs(aEvent.screenX - gMouseDownX) <= 2) {
-    // mouse has moved less than two horizontal pixels between mouse down
-    // and mouse up - call it a click
+  if (!gIgnoreClick && gClickSelectsAll && gURLBar.selectionStart ==
gURLBar.selectionEnd)
     gURLBar.select();
-    gMouseDownX = -1;
-  }
-}

I allowed two pixels of left/right play before selecting all.  You've changed
this to requiring no selection.  I can, though, select a character if you click
in the middle of it and move only one pixel.  Should these two be combined? 
Check that nothing's selected and the mouse has moved less than two pixels
horizontally?

2.
-function URLBarKeyupHandler(aEvent)
-{
-  if (aEvent.keyCode == aEvent.DOM_VK_TAB ) {
-    ShowAndSelectContentsOfURLBar();
-  }
 }

Is this not needed anymore?  Do we handle selecting the url bar when tabbing to
it somewhere else?

3. Does double-click still work when the url bar has focus to select the entire
contents?

4. You removed the blur handler, which did this:

-  // reset the selection so the next time the urlbar is focused it
-  // doesn't re-select the old selection
-  if (gClickSelectsAll && !gIgnoreFocus)
-    gURLBar.setSelectionRange(0, 0);

When I coded 62495 I found that I still needed this - it was there before I
touched anything.  If I remember correctly, it was to ensure everything was
selected when tabbing to the url bar.  Otherwise, if I had just a selection
highlighted and then tabbed off and tabbed back, the selection would remain
highlighted.

Comment 4

16 years ago
Neil, I just noticed that using the context menu key still selects everything. 
Would handling oncontextmenu instead of the right-click address this?
(Reporter)

Comment 5

16 years ago
Dean, thanks for your comments.
What I did was to remove all the urlbar handlers and add back as little as
possible after fixing the context focus event problem. This had the effect of
removing all of your code even if it wasn't directly related to the focus event
problem. I can't observe any problems relating to your third or fifth comments.

Comment 6

16 years ago
Comment on attachment 90282 [details] [diff] [review]
Proposed patch

I tested this and it all looks good.  The fixes for bug 62495 and the
right-click selection bug are maintained, and the remaining bug of the context
menu key selecting everything is also fixed.

You don't need the declaration of gMouseDownX anymore.	Remove that and r=me.

Note to super-reviewer: This doesn't change any existing functionality, it
fixes an underlying problem which I had worked around in my fix for bug 62495.
Attachment #90282 - Flags: review+
(Reporter)

Comment 7

16 years ago
Created attachment 90482 [details] [diff] [review]
Removed gMouseDownX for review

Thanks Dean, good catch on that context menu key!
Attachment #90282 - Attachment is obsolete: true
(Assignee)

Comment 8

16 years ago
Comment on attachment 90482 [details] [diff] [review]
Removed gMouseDownX for review

sr=jag
Attachment #90482 - Flags: superreview+
Attachment #90482 - Flags: review+
(Reporter)

Comment 9

16 years ago
Fix was checked in by bzbarsky@mit.edu
Status: NEW → RESOLVED
Last Resolved: 16 years ago
Resolution: --- → FIXED

Comment 10

16 years ago
Did this first hit 1.3a?  I noticed a change in the behaviour when I installed
Build 2002120208.  Clicking in the URL box beyond the last character of the URL
no longer selects the URL, but instead places the caret at the end of the URL. 
Or was it caused by the Mac changes in bug 116441?  Either way the behaviour
changed and I am finding it irritating.

Comment 11

16 years ago
This changed due to the checkin in bug 116441, and I have reopened that bug,
since that is not the expected behaviour on win32.
You need to log in before you can comment on or make changes to this bug.