User-Agent: Mozilla/5.0 (Windows; U; Windows NT 5.1; de-AT; rv:1.8.1.17) Gecko/20080829 SeaMonkey/1.1.12 Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.1b2pre) Gecko/20081111 SeaMonkey/2.0a2pre Bug 17612 introduced view-source link-browsing for Toolkit/Firefox. The feature itself is working out of the box in SeaMonkey but two things are missing: 1. Go Back / Forward keyboard shortcuts (Alt+Left/Right) do not work 2. There are no Back / Forward entries in the context menu Both work in current Minefield nightlies. Reproducible: Always

confirming, this sounds like a good idea.

Here is a patch that ports the context menu and shortcut keys for view source browsing from Firefox. I also ported the changes from bug 464361.

(In reply to comment #2) > Created an attachment (id=349557) [details] > View source patch > > Here is a patch that ports the context menu and shortcut keys for view source > browsing from Firefox. I also ported the changes from bug 464361. FWIW I can confirm that all three aspects of this patch work. :-)

Comment on attachment 349557 [details] [diff] [review] View source patch I hope I didn't overlook something... >+ <key keycode="VK_BACK" command="Browser:Back"/> >+ <key keycode="VK_BACK" command="Browser:Forward" modifiers="shift"/> You don't actually need all of these. Due to our overlay of navigatorOverlay.xul we've actually got most of the correct stuff hooked up. One thing that I think is missing is the navigationKeys keyset. > document.commandDispatcher.focusedWindow = content; [Hmm, this doesn't seem to work any more.] >+ // Attach the progress listener. >+ getBrowser().addProgressListener(gViewSourceProgressListener, >+ Components.interfaces.nsIWebProgress.NOTIFY_ALL); Actually we're only interested in location changes. >+ getBrowser().removeProgressListener(gViewSourceProgressListener); We don't need to do this, it's a weak reference. >+function HandleAppCommandEvent(event) >+{ >+ event.stopPropagation(); >+ switch (event.command) { We could do Reload too. >+function BrowserForward(aEvent) { >+function BrowserBack(aEvent) { Could you put these before BrowserReload() Also, why the aEvent? >+<!ENTITY backCmd.label "Back"> >+<!ENTITY backCmd.accesskey "B"> >+<!ENTITY forwardCmd.label "Forward"> >+<!ENTITY forwardCmd.accesskey "F"> Can't we get these from contentAreaCommands.dtd?

(In reply to comment #4) > You don't actually need all of these. Due to our overlay of > navigatorOverlay.xul we've actually got most of the correct stuff hooked up. > One thing that I think is missing is the navigationKeys keyset. I cleaned up the commands and the platform-specific keys in this updated patch, but I couldn't get backspace/shift-backspace working when I removed those key definitions despite the fact that they are defined in navigatorOverlay.xul. Adding a BrowserHandleBackspace() function to viewsource.js didn't help, either. > Actually we're only interested in location changes. Fixed. > We don't need to do this, it's a weak reference. Fixed. Since that's all onUnloadViewSource() did, I removed that function from the patch as well. > We could do Reload too. Fixed. > Could you put these before BrowserReload() Fixed. > Also, why the aEvent? Oops, those were unnecessary, so I removed them. > Can't we get these from contentAreaCommands.dtd? Good idea, fixed.

(In reply to comment #5) > but I couldn't get backspace/shift-backspace working when I removed those key > definitions despite the fact that they are defined in navigatorOverlay.xul. They're in a different keyset (but you don't want that one anyway).

Comment on attachment 350261 [details] [diff] [review] View source patch v2 >+ if (aIID.equals(Components.interfaces.nsIWebProgressListener) || >+ aIID.equals(Components.interfaces.nsISupportsWeakReference)) Needs nsISupports too. > function onLoadViewSource() > { > viewSource(window.arguments[0]); > document.commandDispatcher.focusedWindow = content; >+ >+ // Attach the progress listener. >+ getBrowser().addProgressListener(gViewSourceProgressListener, >+ Components.interfaces.nsIWebProgress.NOTIFY_LOCATION); I don't like this separate onLoadViewSource function. Rather than cluttering it up, please can you move this into viewSource() instead. Nit: trailing space. >+ window.addEventListener("AppCommand", HandleAppCommandEvent, true); Please move this up with the other event listeners. r=me with these fixed.

Thanks, carrying over r+. (In reply to comment #7) > Needs nsISupports too. Fixed. > I don't like this separate onLoadViewSource function. Rather than cluttering it > up, please can you move this into viewSource() instead. Fixed. > Nit: trailing space. Fixed. > Please move this up with the other event listeners. Fixed.

Comment on attachment 350440 [details] [diff] [review] View source patch v2.1 [Checkin: Comment 11] Your comment about BrowserHandleBackspace made me wonder whether we should actually move that code into browser.js so that we can share it here.

(In reply to comment #9) > Your comment about BrowserHandleBackspace made me wonder whether we should > actually move that code into browser.js so that we can share it here. Sure, I can take care of that in a followup patch (or a followup bug, if that would be more appropriate).

Pushed changeset f4e096cfccf8 to comm-central. I forgot to mark a=KaiRo on the commit message :-(

Thanks for the checkin. I filed bug 467319 to handle comment #10.