117805 - Context menus don't work in XML documents

Status: VERIFIED FIXED

(Core :: XML, defect)

Description

[Christopher Aillon (sabbatical, not receiving bugmail)]

Linux 2002010206 - 

Right clicking on a document served with an XML mime type will not allow for any
context menus to appear.  The same page served with a text/html mime type will
allow for context menus.  Testcases coming up.

Comment 1

[Christopher Aillon (sabbatical, not receiving bugmail)]

Attachment: XHTML 1.1 served as text/html

Context menu appears in HTML mode

Comment 2

[Christopher Aillon (sabbatical, not receiving bugmail)]

Attachment: XHTML 1.1 served as application/xhtml+xml

Context menu will not appear in XML mode

Comment 3

[basic]

confirmed on w2k with win32 build 2001123008 trunk

Comment 4

[Heikki Toivonen (remove -bugzilla when emailing directly)]

Samir, your checkin to fix bug 15176 broke this. XML document does not have
getSelection() method, it is a method of the interface nsIDOMNSHTMLDocument. Is
there any other way to achieve what you want?

Comment 5

[Samir Gehani]

The reason we changed from _content.getSelection() to
ownerDocument.getSelection() is that the former would not give us the selection
when in documents within a frameset.  If there is some other way to get the
selection with and without frames we should switch over.  

Comment 6

[Christopher Aillon (sabbatical, not receiving bugmail)]

Right clicking brings up these 11 warnings and 3 errors in the JS console:

(11 times)
Warning: reference to undefined property this.target.ownerDocument.getSelection
Source File: chrome://communicator/content/nsContextMenu.js
Line: 636
---------------
Error: this.target.ownerDocument.getSelection is not a function
Source File: chrome://communicator/content/nsContextMenu.js
Line: 636
Error: gContextMenu has no properties
Source File: chrome://messenger/content/mailNavigatorOverlay.xul
Line: 62
Error: gContextMenu has no properties
Source File: chrome://editor/content/editorApplicationOverlay.js
Line: 64

Comment 7

[Christopher Aillon (sabbatical, not receiving bugmail)]

Attachment: Proposed fix v1.0

Comment 8

[Christopher Aillon (sabbatical, not receiving bugmail)]

Taking if you don't mind, Heikki.

The attached patch checks to see if |this.target.ownerDocument.getSelection()|
is a function for the current document.  If it is, we'll use it and if not we
use the older |_content.getSelection()|

Seeking r=/sr=

Comment 9

[Heikki Toivonen (remove -bugzilla when emailing directly)]

Christopher, that is not a full solution. Think about XHTML frameset documents...

Comment 10

[Heikki Toivonen (remove -bugzilla when emailing directly)]

Comment on attachment 64297 [details] [diff] [review]
Proposed fix v1.0

r=heikki

I am in favor of checking this in on the trunk, because the fix to bug 116523
will fix this for real later.

Comment 11

[jag (Peter Annema)]

Comment on attachment 64297 [details] [diff] [review]
Proposed fix v1.0

>Index: resources/content/nsContextMenu.js
>===================================================================
>RCS file: /cvsroot/mozilla/xpfe/communicator/resources/content/nsContextMenu.js,v
>retrieving revision 1.48
>diff -u -r1.48 nsContextMenu.js
>--- resources/content/nsContextMenu.js	2001/12/21 00:43:53	1.48
>+++ resources/content/nsContextMenu.js	2002/01/10 17:19:26
>@@ -633,7 +633,11 @@
>     },
>     
>     searchSelected : function() {
>-        var searchStr = this.target.ownerDocument.getSelection();
>+        var searchStr;
>+        if (typeof this.target.ownerDocument.getSelection == "function")

A better way of doing this would be:
  if ("getSelection" in this.target.ownerDocument)

Could you add a comment pointing to bug 117805 and bug 116523, and add
a comment to the latter bug mentioning this change can be undone once
that is fixed?

Comment 12

[Christopher Aillon (sabbatical, not receiving bugmail)]

Attachment: Patch v.2 with jag's changes

Comment 13

[jag (Peter Annema)]

Comment on attachment 64975 [details] [diff] [review]
Patch v.2 with jag's changes

sr=jag

Carrying over Heikki's r=, which I'm sure still stands.

Comment 14

[Henrik Gemal]

isn't getSelection deprecated?

Comment 15

[Christopher Aillon (sabbatical, not receiving bugmail)]

Henrik, yes that is bug 116523 which this bug depends on -- this is just a
workaround to get context menus back in XML.

attachment 64975 [details] [diff] [review] has been checked in by bbaetz.  Heikki should we leave this
open for the real fix?  Or just check back after Samir fixes 116523?

Comment 16

[Heikki Toivonen (remove -bugzilla when emailing directly)]

Let's mark this fixed and keep eye on Samir when he fixes bug 116523, it will
HAVE to work on XML as well... 

Comment 17

[Christopher Aillon (sabbatical, not receiving bugmail)]

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

Comment 18

[Chris Petersen]

Marking verified.