[ESM/CSS] Right-click != Left-click

VERIFIED FIXED in mozilla1.0.1

Status

()

P2
major
VERIFIED FIXED
19 years ago
17 years ago

People

(Reporter: bugzilla, Assigned: bugzilla)

Tracking

Trunk
mozilla1.0.1
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [nsbeta3-], URL)

Attachments

(6 attachments)

(Reporter)

Description

19 years ago
(Almost) all over Mozilla you can right-click on a Ok or Cancel or other buttons 
and the button gets activated.

EX: go into preferences. Right-click on Ok and your prefs get saved!

This should NOT be the case! Right-clicking on a button is not the same a 
left-clicking!

Expected:
The right-click should be ignored!

Comment 1

19 years ago
reassigning to evaughan as p2 for m16, since this could plausibly have
unintended bad consequences.
Assignee: trudelle → evaughan
Priority: P3 → P2
Target Milestone: M16

Comment 2

19 years ago
*** Bug 30875 has been marked as a duplicate of this bug. ***

Comment 3

19 years ago
*** Bug 20020 has been marked as a duplicate of this bug. ***

Updated

19 years ago
Status: NEW → ASSIGNED
Target Milestone: M16 → M17

Comment 4

19 years ago
Above, it says that "bug #20020 has been as a duplicate" but on bug #20020
the corresponding comment/resolution-change has not been made. 

They could certainly be marked as duplicates, but evaughan was accepting 20020
at the same time that this duplicate annotation was added here. (?)

(cc: terry if he wants to investigate how this happened). 

Comment 5

19 years ago
*** Bug 20020 has been marked as a duplicate of this bug. ***

Comment 6

19 years ago
Mass moving M17 bugs to M18
Target Milestone: M17 → M18

Comment 7

19 years ago
Tom,

How do we handle this? A button moves to the active state whether or not you 
press the right or left button. CSS shows a depressed look when :active. How 
can we fix this?

Comment 8

19 years ago
I think we need some kind of new pseudo styles. Right now we have :active but we 
really need 

:active1 - active only when the left button is pressed
:active2 - active only when right button is pressed

and then :active would be left or right.

Comment 9

19 years ago
mass-moving all bugs to m21 that are not dofood+, or nsbeta2+
Target Milestone: M18 → M21

Comment 10

19 years ago
Tom how do we fix this? It probably happens in html as well.
Assignee: evaughan → joki
Status: ASSIGNED → NEW

Comment 11

19 years ago
*** Bug 43761 has been marked as a duplicate of this bug. ***

Updated

19 years ago
Status: NEW → ASSIGNED

Comment 12

19 years ago
Adding [ESM/CSS] prefix to bugs in EventStateManager with its generated events 
or CSS pseudo-class management.
Summary: Right-click != Left-click → [ESM/CSS] Right-click != Left-click
(Assignee)

Comment 13

18 years ago
*** Bug 45533 has been marked as a duplicate of this bug. ***

Comment 14

18 years ago
This bug has been marked "future" because the original netscape engineer working 
on this is over-burdened. If you feel this is an error, that you or another
known resource will be working on this bug,or if it blocks your work in some way 
-- please attach your concern to the bug for reconsideration.
Target Milestone: M21 → Future
I thought this was buttons only, but it does not seem to be. If I have onclick 
handler on some HTML element (tested input text field and textarea), the onclick 
handler gets called! IE5 gets it right.

Reassigning to me. Removing Future milestone and nominating for nsbeta3.
Assignee: joki → heikki
Status: ASSIGNED → NEW
Keywords: nsbeta3
Target Milestone: Future → ---
Triage with nisheeth & ekrock: nsbeta3+.
Whiteboard: [nsbeta3+]
There is a related bug about onclick handler and context menu, it is bug . From 
that bug we could shamelessly copy the proposal of zach@math.berkeley.edu to 
prevent us from processing all but left clicks on dialog buttons quite easily. 
First, we should remove the onclick handlers in platformOverlay.xul:

<button class="exit-dialog" id="cancel" value="&cancelButton.label;" 
onclick="doCancelButton()"/>
xxxxxxxxxxxxxxxxxxxxxxxxxx

Then in dialogOverlay.js, for each button in the dialog we could create an event 
handler function:

function cancelClickHandler(e) {
  if(e.button == 1)
    doCancelButton();
}

Finally attach the event handlers to the buttons, in doSetOKCancel 
(dialogOverlay.js):

  var cancelButton = document.getElementById("cancel");
  if (cancelButton) {
    okButton.addEventListener("click",clickHandler,false);
  }
Created attachment 13002 [details] [diff] [review]
Fix dialog default buttons to act on left-click only
I made a patch if anyone wants to give this a try. 

There is still a problem with the Macs, though, because they do not use onclick 
handlers. Also, this patch does nothing with the style, eg. you will get pressed 
down look for the button even with a right click.
There is also a simpler way that does not concern the Macs I believe. In 
platformDialogOverlay.xul we could change the onclick handlers to look like 
this:

onclick="if(event.button==1)doOKButton()"
         +++++++++++++++++++

The problem seems to be that the whole GUI is full of onclick handlers. Fixing 
the default buttons is not much work, but most other things also respond to 
right click. There are also exceptions that get it right.
Status: NEW → ASSIGNED
Ben, what are your thoughts about going through the GUI and changing controls so 
that they act on left-click only?
Whiteboard: [nsbeta3+] → [nsbeta3+][partial fix]
Quick grep for onlick in XUL files on Windows gave 422 instances. Should we add:

if (event.button==1)

to all of them, or some selected like Ok & Cancel, or just future this bug? Of 
course each use would need to be checked first if we want to fix those things...

About onclick getting called for HTML: I guess that is correct after all, you 
can get the button that was pressed from event.button. There are problems with 
context menus which are addressed in bug bug 28604.

Comment 24

18 years ago
imo The fix should be done in c++ code not xul.
ERM
do we have onCommand for buttons? if so does it only use primary mouse button? 
If so maybe that is the solution.

Hyatt do you do anything xul/xbl/button related?
oncommand seems to work for buttons as well. Still there are things that are not 
affected by this, for example checkboxes generally don't seem to have onclick 
handlers so they would still respond to right-click...

I did a blind search-replace for onlick in XUL & JS chrome, and that left at 
least the sidebar crippled (couldn't change tabs).

Comment 26

18 years ago
This has to do with the nsButtonBoxFrame.cpp's MouseClicked handler.  Most XUL 
widgets don't use onclick, but instead use oncommand.  I need to not fire that 
on a right click.  (Right now I do.)
Actually, it is not nsButtonBoxFrame.cpp's MouseClicked. That creates an 
"command" event only on mouse left click. But it is already too late in the game 
anyway, the script handlers have been called already.

It seems XBL can also help here. At least for checkboxes we have, in 
xulBindings.xml, a line that says:

<handler type="click" value="this.checked = !this.checked;"/>

If I add the attribute button="1" or change type from "click" to "command"
then nsXBLEventHandler::MouseEventMatched(nsIDOMMouseEvent* aMouseEvent) will 
filter all but left-clicks. Now XBL is totally uncharted terrotory for me, so I 
am not sure if this would cover all the XUL (checkboxes, radiobuttons, normal 
buttons, menus, drop-down lists and whatever). I guess it could.

This also brings up a couple of issues. First, the documentation at

http://www.mozilla.org/xpfe/xptoolkit/xbl.html

is a wrong at least in regard with the button attribute. The docs say you can 
use values left, middle and right, but code checks for numbers.

Also, there are 2 xulBindinds.xml files in the source tree, in 
xpfe/global/resources/content and ... skin.

Is anyone willing to take this stuff since this doesn't seem to be event stuff, 
or should I go and learn XBL?
In my opinion this is not an event system bug. It could be "fixed" in event 
code, perhaps, but I think the correct way is to fix the XBL and XUL files. 
Therefore, reassigning to Trudelle for further triage...
Assignee: heikki → trudelle
Status: ASSIGNED → NEW

Comment 29

18 years ago
->hyatt, based on his last comment. clearing nsbeta3+ for triage by current owners
Assignee: trudelle → hyatt

Comment 30

18 years ago
I am reassigning this to ben, but this problem is huge and really can't be 
covered by one bug.

The big problem is that most people still use onclick for buttons and they don't 
check the button type.  I designed oncommand intending for people to always use 
it instead of onclick for buttons, since oncommand filters to the left button.

Ben, you can add button="1" to checkbox and radio bindings to fix those, but 
still, anyone who puts an onclick handler on these without checking the button 
is going to get in trouble.

We need to have a mass onclick-->oncommand spree with the XUL.
Assignee: hyatt → ben
I've fixed this in the simple case for checkboxes and radiobuttons by checking 
event.button checks. The button problem remains. Removing nsbeta3+ and 
reassigning back to hyatt. 
Assignee: ben → hyatt
Whiteboard: [nsbeta3+][partial fix] → [partial fix]

Comment 32

18 years ago
Ben, why exactly did you give this back to hyatt?  Is there a remaining problem
with button, or just with the incorrect use of onclick in buttons?  

Comment 33

18 years ago
fwiw, I have patches for all of the onclicks I found using lxr.  I sent them to 
hyatt. Full diffs are available in the url. I think I need module owner reviews 
or something like that.
Keywords: approval, patch, review
Whiteboard: [partial fix] → [partial fix][url has diffs for full fix, need module review/approval]

Comment 34

18 years ago
Why Hyatt?  Who owns all of those onclicks?

Comment 35

18 years ago
->ben
Assignee: hyatt → ben
Timeless: please attach your patch.  Ben: please give it an r= if you can, or
say who else should review it to get it in.  Thanks,

/be

Comment 37

18 years ago
nav triage team:
nsbeta3- not a show stopper
Whiteboard: [partial fix][url has diffs for full fix, need module review/approval] → [nsbeta3-][partial fix][url has diffs for full fix, need module review/approval]
(Reporter)

Comment 38

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

Comment 39

18 years ago
*** Bug 55672 has been marked as a duplicate of this bug. ***
=> hyatt. oncommand should fire on left click only. 

(I have a feeling this has been bounced about before ;)
Assignee: ben → hyatt

Comment 41

18 years ago
Oncommand does fire on left click only.  This bug is about people using onclick
handlers when they should use oncommand.

Assignee: hyatt → ben
(Reporter)

Comment 42

18 years ago
Will we see a checkin soon?
(Assignee)

Comment 43

18 years ago
I just filed bug 62256, but I guess it's really a dup of this, so I'll take 
this.  I don't understand the attached patch; why use onclick and then filter 
to button 1 (which will also break the case that I filed 62256 for) rather than 
just using oncommand?  I'll attach a patch soon with a generic 
s/onclick/oncommand for buttons.  I can't think of any case where a button 
would want onclick, but I'll be careful as I go...
Assignee: ben → blakeross
(Assignee)

Comment 44

18 years ago
*** Bug 62256 has been marked as a duplicate of this bug. ***

Comment 45

18 years ago
One problem, as i was going through this patch (a while ago), i found a few 
objects that did not fire oncommand events.  Also, by now my patch has 
certainly rotted.

Clearing keywords and nominating for 0.9 and 1.0.

fwiw, the diffs are still available in the url if anyone really wants to try 
them.
Keywords: approval, nsbeta3, patch, review → mozilla0.9, mozilla1.0
OS: Windows 98 → All
Hardware: PC → All
(Assignee)

Comment 46

18 years ago
Created attachment 20313 [details] [diff] [review]
[patch][1] some XPApps overlays

Comment 47

18 years ago
r=jag
(Assignee)

Comment 48

18 years ago
cc alec for sr

[anyone getting notifications: I'm going to be attaching a number of patches in 
different areas so the appropriate people can review each.  please feel free to 
remove yourself if you don't want the spam.]
Status: NEW → ASSIGNED
Keywords: mozilla0.9, mozilla1.0 → nsbeta3
Whiteboard: [nsbeta3-][partial fix][url has diffs for full fix, need module review/approval] → [nsbeta3-]
Target Milestone: --- → mozilla1.0

Comment 49

18 years ago
sr=alecf
(Assignee)

Comment 50

18 years ago
Created attachment 20413 [details] [diff] [review]
[patch][2] editor files

Comment 51

18 years ago
RCS file: /cvsroot/mozilla/editor/ui/dialogs/content/EdImageMap.xul,v
you didn't add ; after ().
(Assignee)

Comment 52

18 years ago
r=kin via e-mail, cc simon for sr

Comment 53

18 years ago
sr=sfraser. You should get r= from cmanske on the editor changes.
(Assignee)

Comment 54

18 years ago
Charley already looked them over.  Thanks Simon.
(Assignee)

Comment 55

18 years ago
Created attachment 20951 [details] [diff] [review]
[patch][3] mail/news files
the tree has shifted under blake.  blake, can you attach a new mailnews patch?
ok, blake emailed me the update.  

r=sspitzer

blake, you'll should attach it for the super reviewer.

Comment 58

18 years ago
sr=alecf
(Assignee)

Comment 59

18 years ago
Created attachment 21018 [details] [diff] [review]
[patch][4] cookie/wallet files

Comment 60

18 years ago
as before, sr=alecf as long as each button has been tested

Comment 61

18 years ago
WalletEditor.xul is no longer being used, but the fix there is certainly 
harmless.

However, I think you missed a file.  How about SignonViewer.xul?

r=morse for the files presented.  And the r= applies to the SignonViewer file as 
well if that will be the same sort of change.   
(Assignee)

Comment 62

18 years ago
Created attachment 21042 [details] [diff] [review]
[patch][5] chatzilla file

Comment 63

18 years ago
r=rginda for patch 5
(Assignee)

Updated

18 years ago
Blocks: 49773
(Assignee)

Comment 64

17 years ago
Marking fixed. Pretty much all cases have been fixed.
Status: ASSIGNED → RESOLVED
Last Resolved: 17 years ago
Resolution: --- → FIXED
(Reporter)

Comment 65

17 years ago
what about right clicking on the "Get Msgs" and "Print" button in the Mail&News?
also the entire addressbook
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
(Assignee)

Comment 66

17 years ago
Umm...I don't know what you're talking about, but file new bugs to appropriate
components.
Status: REOPENED → RESOLVED
Last Resolved: 17 years ago17 years ago
Resolution: --- → FIXED
(Reporter)

Comment 67

17 years ago
What I meant was:
If you right click on the "Get Msgs" and "Print" button in the Mail&News the
button seems to get clicked. It doesn't do anything but the button seems to get
pressed down. It only happens to those two button because they both have some
sort of dropdown function. It doesn't happen on the "File" button which also
have some dropdown function.

should I file new bugs or if so what component is that?

Status: RESOLVED → REOPENED
Resolution: FIXED → ---

Comment 68

17 years ago
Bugs targeted at mozilla1.0 without the mozilla1.0 keyword moved to mozilla1.0.1 
(you can query for this string to delete spam or retrieve the list of bugs I've 
moved)
Target Milestone: mozilla1.0 → mozilla1.0.1
(Assignee)

Comment 69

17 years ago
That has nothing to do with this bug.
Status: REOPENED → RESOLVED
Last Resolved: 17 years ago17 years ago
Resolution: --- → FIXED
(Reporter)

Comment 70

17 years ago
v 20020221
will file new bugs about weird rightclick != leftclick stuff
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.