236666 - Add context menu pref to Advanced Javascript Options

Status: RESOLVED FIXED

(Firefox :: Settings UI, enhancement)

Description

[PikeUK]

User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.7b) Gecko/20040306 Firefox/0.8.0+
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.7b) Gecko/20040306 Firefox/0.8.0+

This is to request that a checkbox be added in the Advanced Javascript Options
to prevent websites from hiding the context menu (see Bug 86193 for backend
implementation). This is the Firefox equivalent of Bug 117532 (I couldn't find
an existing Firefox equivalent).

Reproducible: Always
Steps to Reproduce:




Will attach a possible patch in next comment.

Comment 1

[PikeUK]

Attachment: Add Hide Context Menu checkbox to Advanced Javascript Options

This adds a checkbox to the advanced javascript options box to turn on/off the
pref from Bug 86193.

I don't know if this is the right way to do it but it works for me. The
existing pref window code assumes that true = on and false = off, unfortunately
the new context menu pref is the reverse of this, so I copied the prefinverse
code from the xpfe version of nsPrefWindow.js and used that instead.

p.s. I had some problems patching pref-advancedscripts.xul, the patch would
always give a hunk failed when applied. I narrowed it down to being caused by
the blank spaces at the end of the file, removing them in the patch seemed the
only way to avoid the hunk failed errors.

Comment 2

[Mike Connor [:mconnor]]

pch already wanted to port the prefinverse stuff already (see bug 225476).  He'd
probably be a good person to ask for review from.

Comment 3

[Robert Accettura [:raccettura]]

Attachment: Firefox patch from bug 117532

No idea if it's a good patch or not, it was me learning some of firebird's
preferences.  But I need to close bug 117532.  So I'm dumping it here ;-).

Comment 4

[Jeremy M. Dolan]

Has the idea to have a hotkey+click override javascript events been canned? To
use this pref requires a user to submit to silently hiding site functionality. I
don't think any of the other prefs in JS->Advanced qualify as functionality
losses. This is, however, certainly better than nothing.

Comment 5

[Robert Pongetti]

I'm a Firefox newbie, running 0.8 on Windows 2000.

As mentioned on other bugfix pages (86193) and other pages around the net, I
added a boolean preference through about:config titled
"dom.event.contextmenu.enabled" and set it to false, to no avail.  My
right-clicks still don't work on, for example, these sites:

http://www.dynamicdrive.com/dynamicindex9/noright.htm
http://www.mailmontage.com/index_test.html
http://bugzilla.mozilla.org/attachment.cgi?id=143517&action=view

Just to make sure I wasn't doing something wrong, I check my pref.js file, and
it contains:

user_pref("dom.event.contextmenu.enabled", false);

I don't know how to install a patch, so I haven't tried that.  I'm sure its
something incredible simple that I'm missing entirely, even after Google
searches on the subject.

I even tried installing a nightly build, and it wasn't in there.

Any idea how to make this fix applicable?

Comment 6

[Robert Accettura [:raccettura]]

This patch is still awaiting review.

Comment 7

[Robert Pongetti]

Is there some kind of preference I can manually set to always enable my
right-clicks until the javascript option patch is complete?

I tried setting dom.event.contextmenu.enabled to false and it didn't seem to help.

Comment 8

[Robert Pongetti]

Nevermind.

I just figured out that you had to set it for every page you want to enable
right-clicks on.  The reset is now bookmarked.

Comment 9

[Robert Pongetti]

Sorry for the multiple posts, but I just ran accross an interesting issue for
this bugfix overall that has nothing to do with my prior posts.

As noted above, I've got the Enable Context Menu javascript bookmarked (should
do fine until you guys finish the javascript patch), and it works marvelously to
allow a right-click context menu to pop up for the follow test pages cited on
parallel bugzilla pages:

http://www.dynamicdrive.com/dynamicindex9/noright.htm
http://www.mailmontage.com/index_test.html
http://bugzilla.mozilla.org/attachment.cgi?id=143517&action=view

HOWEVER, I found a page that zaps even that fix.  When I am reading any legal
case (e.g., Brown v. Board of Education) on a pay legal service called
"Westlaw," right-clicking on the links to other case cites in the text of the
case I'm reading automatically opens a "link viewer" with many less options.  I
am given no option to open the link in a new window or tab (in fact, I have no
right-click at all).  Its not a flash player or anything of the sort.  Note that
opening the "link viewer" is the same thing it does when those links are
left-clicked.

Oddly, using Internet Explorer with zero hacks, I can right-click on these
Westlaw pages and open them in new windows.   In fact, I've never had a problem
with it before.  It just opens the case I want to view in a normal browser
window, with additional options.

I wish I could give you a link, but its a pay legal service so expensive that no
one in their right mind (and that, of course, excludes lawyers) would use it.

If you want me to test it with any fix, I'd be glad to if someone emails me.  I
could also save a page as an "mht" file in IE and send it to someone.

Comment 10

[Robert Parenton]

Only builds after 03/05/2004 18:58 PST have the functionality that this bug
would provide a UI for.  Also, this is not the correct bug for any of comments
5,7,8, and 9.

Comment 11

[Mike Connor [:mconnor]]

not a blocker, but one patch or the other would be nice for 0.9

Comment 12

[Steffen Wilberg]

The "prefinverse" attribute was replaced by the "reversed" attribute, see bug
181973 for that as well. I suggest to do the same here.

Comment 13

[PikeUK]

Attachment: Add Hide Context Menu checkbox (now using reversed attribute)

Thanks Steffen I spotted that yesterday, here's the updated patch to use
reversed instead (which we already support with checkboxes so no need to add it
this time).

Comment 14

[Steffen Wilberg]

*** Bug 242121 has been marked as a duplicate of this bug. ***

Comment 15

[Steffen Wilberg]

Comment on attachment 144016 [details] [diff] [review]
Firefox patch from bug 117532

That's obsolete, right?

Comment 16

[Mike Connor [:mconnor]]

Comment on attachment 147418 [details] [diff] [review]
Add Hide Context Menu checkbox (now using reversed attribute)

wouldn't fixing listitems to use reversed and just using reversed on the
allowHideContextMenu pref make more sense? I'm not sure I'm reading this right,
which is why this isn't a review :)

Comment 17

[Steffen Wilberg]

No, it wouldn't. The prefs used to have the opposite meaning than the UI
elements, like "dom.disable_window_move_resize", which corresponds to "Allow
scripts to move or resize existing windows". So we have to do a negation between
the two.

Currently, we're doing this in onDialogLoad and onDialogAccept. But now we've
got a pref which doesn't need to be negated: "dom.event.contextmenu.enabled". So
we've got to differentiate between these prefs. This is easily done by using the
reversed attribute on the prefs which need to be negated.

Comment 18

[Bernard Alleysson]

There's a patch and it can generate some good buzz or reviews, I mean more stuff
like "It's all about YOU".

Comment 19

[Mike Connor [:mconnor]]

Comment on attachment 147418 [details] [diff] [review]
Add Hide Context Menu checkbox (now using reversed attribute)

Change the wording to "Disable or replace context menus" and r=me

from there, HideContextMenu should be something more like "ControlContextMenu"
for clarity

Comment 20

[PikeUK]

(In reply to comment #19)
> (From update of attachment 147418 [details] [diff] [review])
> Change the wording to "Disable or replace context menus" and r=me
> 
> from there, HideContextMenu should be something more like "ControlContextMenu"
> for clarity
> 

OK, but doesn't "Disable or replace the context menu" sound better? I'll attach
an updated patch shortly.

Comment 21

[PikeUK]

Attachment: Add Hide Context Menu checkbox (address mconnor's comment)

Menu name changed per comment 19.

Comment 22

[Mike Connor [:mconnor]]

checked in branch and trunk 2004-05-28 10:30

Comment 23

[Mike Connor [:mconnor]]

sorry for bugspam, long-overdue mass reassign of ancient QA contact bugs,
filter on "beltznerLovesGoats" to get rid of this mass change