win32 build 2001122103, win98se This shows in the js console after right-clicking on a link, image, or highlighted text: Deprecated method document.getSelection() called. Please use window.getSelection() instead.
Confirming on same build/OS. Appeared with this build, I think.
Status: UNCONFIRMED → NEW
Ever confirmed: true
hm, should this go to selection...?
Assignee: pchen → trudelle
OS: Windows 98 → All
Hardware: PC → All
Assignee: trudelle → blaker
Assignee: blaker → blaker
-> sgehani added the above in rev 1.48 of nsContextMenu.js
Assignee: blaker → sgehani
The fix for this should take care of the symtoms in bug 117805 as well.
Priority: -- → P2
Target Milestone: --- → mozilla0.9.9
made a fix for this
Assignee: sgehani → Morten
this could very well be a dupe of bug 117805, but I'm leaving it as is for a while.
I doubt this patch works in documents with framesets so unless you propose to get to the right fix before me (I've targetted mozilla0.9.9) please reassign this back to me.
well.. I just tested this patch with a frameset.. located here: http://home.no.net/blan01/ and the context menu displayed the right items... if you give me some urls to test against, I'll do that...
Actually, all the context menu items are not displayed. The |Search Google for "<selected text>"| menu item (of course your default search engine will show in place of Google) does not display cause window.getSelection() doesn't return the text selected in the frame. But thanks for forcing me to test and confirm comment 11. Comment 11 still holds so please advise accordingly whether I should take this bug. Thanks.
Morten Nilsen, no that patch is not correct for the same reason the patch which I attached on bug 117805 is not correct either. It's a workaround, not the fix. The reason we are using getSelection in the first place is for getting the selected text. Highlight some text, and then right click and you will see a item that says something to the effect of "Search this page for 'selected text'" The reason we switched to this.target.ownerDocument.getSelection() is because of it not working in framesets. The only reason i attached the patch on bug 117805 is because the search feature not working on XHTML frameset docs is much better no context menus at all in XML documents. This patch was meant to only fix a console error, and didn't take into consideration the feature which is behind this. If anything this patch should be obsoleted by the patch on bug 117805. That patch is intended for 0.9.8 branch maybe. I don't know if it will get checked into the trunk. That patch at least allows for HTML framesets to work with the search selected text feature, but XHTML as XML frameset docs won't work still (although context menus will). This nor the patch on bug 117805 is the proper fix. The one on 117805 is better, but let's get the real fix in. If you still feel you want to own this bug and fix it the real way, go ahead. Marking this bug as blocking 117805.
I have never seen any context menu item with search in it. neither before nor after making this patch... but I'm giving it back to Samir... I don't think I'll be able to do anything more about this.
Assignee: Morten → sgehani
Status: ASSIGNED → NEW
Morten, The search context menu item only shows up when you highlight (i.e., select) text and then right click (which is a discoverability glitch that needs addressing by always leaving the item visible and only enabling it when text is selected).
Spilling over into mozilla1.0.
Target Milestone: mozilla0.9.9 → mozilla1.0
*** Bug 126775 has been marked as a duplicate of this bug. ***
is there no way to get the window of the document? i fixed this bug by coding a frameset-walker funtion, which searches the frameset for the window which contains the document where the right click happened... is this too stupid or should i attach that?
Created attachment 71923 [details] [diff] [review] proposed patch Fabian told me a magic thing... "__parent__" great.. changing that one line removes the warning...
Created attachment 72023 [details] [diff] [review] proposed patch v2 this is even better... as we don't need the workaround any more... (at least that's how i understood this) and i checked with the attachment in bug 117805 which works fine too... (even in a frameset) so... this obsoletes attachment 71923 [details] [diff] [review] ...but i can't do that...
Created attachment 72039 [details] [diff] [review] proposed patch v3 new version because i just tested with a xul document and that throws an error that this.target has no properties... that's true also for the the current state. sorry for the patch mess but it never worked in xul since the if was inserted... and i didn't know that... this patch works with xhtml frames/noframes, html frames/noframes and xul and never throws any warning or error
Moving Netscape owned 0.9.9 and 1.0 bugs that don't have an nsbeta1, nsbeta1+, topembed, topembed+, Mozilla0.9.9+ or Mozilla1.0+ keyword. Please send any questions or feedback about this to email@example.com. You can search for "Moving bugs not scheduled for a project" to quickly delete this bugmail.
Target Milestone: mozilla1.0 → mozilla1.2
Comment on attachment 72039 [details] [diff] [review] proposed patch v3 r=sgehani
Attachment #72039 - Flags: review+
firstname.lastname@example.org, You need a super-review and approval from email@example.com. I can checkin the patch once you have that. See: <http://www.mozilla.org/hacking/reviewers.html>.
Assignee: sgehani → rossi
Keywords: patch, review
Priority: P2 → --
Target Milestone: mozilla1.2 → ---
Comment on attachment 72039 [details] [diff] [review] proposed patch v3 This won't work. It causes more warnings than we start with on XHTML+XML frameset documents. Select and right-click on any of the frames here to test. http://www.returnzero.com/frameset.xhtml
Attachment #72039 - Flags: needs-work+
Created attachment 74451 [details] [diff] [review] The Real Fix Use |document.commandDispatcher.focusedWindow| instead of what we were using. Man, I sure wish I had known about this when I did the fix for 117805. This is definitely what we need. Tested that the 'Search Netscape Search for "your selection"...' feature works on HTML documents, HTML frameset documents, XML documents, and XML fraeset documents. No strict warnings appear at all, either. <QA> HTML document: (use this page) HTML frameset document: http://www.usahistory.com/frames.htm XML document: http://bugzilla.mozilla.org/showattachment.cgi?attach_id=63292 XNK frameset document: http://www.returnzero.com/frameset.xhtml </QA>
Attachment #72039 - Attachment is obsolete: true
Comment on attachment 74451 [details] [diff] [review] The Real Fix looks great =)
Attachment #74451 - Flags: review+
Taking to seek super review and approval. This fix also enables the search selected text context menu (bug 15176) in XHTML as XML frameset documents, which was previously not working. Updating summary.
Assignee: rossi → caillon
Summary: right-click uses deprecated method document.getSelection() → Search selected context menu item missing in XHTML as XML framesets, right-click uses deprecated method document.getSelection()
Comment on attachment 74451 [details] [diff] [review] The Real Fix sr=jag
Attachment #74451 - Flags: superreview+
Comment on attachment 74451 [details] [diff] [review] The Real Fix a=asa (on behalf of drivers) for checkin to the 1.0 trunk
Attachment #74451 - Flags: approval+
cvs commit -m "Bug 116523, Search selected text context menu missing in XHTML as XML frameset docs, strict warning on right click. firstname.lastname@example.org, sr=jag, a=asa" nsContextMenu.js Checking in nsContextMenu.js; /cvsroot/mozilla/xpfe/communicator/resources/content/nsContextMenu.js,v <-- nsContextMenu.js new revision: 1.59; previous revision: 1.58 done Attachment 74451 [details] [diff] checked in on trunk.
Status: NEW → RESOLVED
Last Resolved: 16 years ago
Resolution: --- → FIXED
vrfy'd fixed using 2002.04.09 comm bits on linux rh7.2, win2k, mac 10.1.3. tested with Christopher's tests in comment 27.
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.