63026 - Give me the menu in View Source back

Status: VERIFIED FIXED

(SeaMonkey :: UI Design, defect)

Description

[Ben Bucksch (:BenB)]

View Source once had a menu. Currently, it doesn't. This is bad, especially
since the Find commmand is unavailable because of that. The Find command is very
important in order to locate the source of a certain part of the page,
considering the crap most commercial websites generate around the content. It is
also important to check non-existance of certain strings, e.g. "doubleclick".

<quote src="bug 21837">
------- Additional Comments From [e-mail] 1999-12-20 11:16 -------
Actually, the view source window SHOULD have chrome, especially menus so you can
access the find command in particular.  The 4.x design really sucks. :-)

------- Additional Comments From Ben Goodger 1999-12-20 13:24 -------
just tried this in a build from a couple of days ago, view source window has
menus, and a taskbar. This is as it should be, and as web developers have wanted
for years, so I'm not gonna fix what ain't broke ;)
</quote>

Please put the menu back, at least a minimal version. (BTW: I *do* think that
View Source of View Source makes sense :-P .)

Comment 1

[Jesse Ruderman]

See also:
bug 50877 Mac problems caused by the lack of menus in View Source
bug 39389 View Source needs context menus

Btw, Ctrl+F does work in View Source, but that doesn't mean that View Source 
doesn't need menus.

Comment 2

[Jesse Ruderman]

... and also see the old "menus missing from view source" bug, bug 32719, which 
was marked as a dup of a bug to make keyboard shortcuts work in the view source 
window.

Comment 3

[Blake Ross]

Working on it.

Comment 4

[Paul Chen]

nav triage team:
Netscape folks probably won't fix. Marking nsbeta1-; however, that doesn't mean 
it isn't important!

Comment 5

[Doron Rosenberg (IBM)]

ok, i was playing with xul and created a working model, just need some sort of
spec for this.

currently i have:
File >> Save As | Close
Edit >> Copy | Select All
Search >> Find in this Page | Find Again

an addtion perhaps would be edit page in File, but i was having trouble with
that (editor was complaing about something).

I currently have all the text in viewSource.dtd, however, I could probably load
these from other dtd's as well.

Comment 6

[Matthew T (active 1999-2002)]

Methinks the menus for a View Source window should be practically identical to 
those for a browser window, with the exception that in the View Source window the 
`View' and `Go' menus would be mostly disabled.

Comment 7

[Jesse Ruderman]

mpt: if View Source has the same menus as Navigator, I think it would make 
sense to actually make Navigator and View Source the same window (bug 12111).  
That can be done later, though.

Comment 8

[Doron Rosenberg (IBM)]

mpt: really? interesting.  Bookmarks and such as well?

Comment 9

[Matthew T (active 1999-2002)]

Well, heh, that depends on what platform you're using ... On Mac OS, you 
usually have the same set of menus for all windows in an app, and just disable 
the ones which can't be used from the current window -- and there's no reason 
not to open bookmarks (in new browser windows) from a View Source window. On 
Windows, however, you just show the menus which are applicable to the current 
window.

Comment 10

[Doron Rosenberg (IBM)]

i'll be gone till march, will get this checked in then (it's almost complete).

Comment 11

[Paul Chen]

Actually, if you stick in a menubar via an overlay, you'll force the mac to get
that menubar, too. Right now, view source gets the navigator menubar on mac (I
think because the hidden window has that menubar), but it doesn't have xul to
disable menu items, so you get menu items that don't work, which is bug 50877. I
plan on fixing 50877 by adding xul to hidden window to fix mac. I could just do
the same thing for view source, which would put the navigator menu bar on view
source.

Comment 12

[Sean Richardson]

Here's a proposed set of menu items for the View Source window, with an eye to
(a) ensuring maximum accessibility for those who cannot use a mouse, and
(b) adding functionality missing from NN4.x and 6.0 that could be used with
the page source itself:

Minimal Menus for View Source:

File                Edit        View              Search                  Help	

# New Navigator    (Undo)       * Text Size ->    # Find in Page Source
-------------	   (Redo)       --------------    # Find Again	
* Close            -------      # Page Info	
# Save Source as   (Cut)        ------------	
--------------     * Copy       * Character Coding ->
# Edit Page        (Paste)      
--------------     (Delete)
# Page Setup        --------
# Print Source     * Select All

Legend: 
(item): greyed/nonfunctional   
* item: for accessability
# item: for additional functionality

Notes:
1. On the Edit menu, only Copy and Select All should work, but the rest of the
items should remain for consistency and ease of menu naviagtion.

2. On the View menu, the Character Coding submenus would be *necessary* to read 
the text among the HTML code for non-latin-alphabet languages, if View Source 
does not "inherit" the character coding of the page. If it does, it might
still be a worthwhile convenience should there be more than one coding on a
page. Might be hard to implement, though; if so, better to drop it.

3. Yes, an editor (any editor) would be a better environment for printing
source, but if the user wants to, why not -- just so long as it isn't the
layed-out page that gets printed from the print item in View Source!

Matthew's suggestion of keeping almost the full Navigator menus in View source
on the Mac makes sense, so long as most of the Edit menu gets disabled too.
But as easy as it would be to make that XP, it *would* be wrong for Windows.

Comment 13

[timeless]

'Save Source as' should be 'Save As'
'Find in Page Source' should be 'Find'
'Page Info' should be 'Document Info' /*you're reading the source of a 
document, it may or may have any relation to the concept of a web page (ideally 
you could be looking at a svg or js file)*/

we should also have: File>Edit Page, View>Reload

Comment 14

[Doron Rosenberg (IBM)]

edit page works btw, I'll get to this once I can get xfree to cooperate with my
new graphics card as the old one died.

Comment 15

[Doron Rosenberg (IBM)]

ok, I have it done, need to feature clarification:

- should the tasks menu be included?
- as an sideeffect, both open web location and open file work in viewsource -
should they be included into the File menu or remain hidden features only
accesible with a key combination?
- page info obviously gives me no usefull information, as the page is never
parsed.  Perhaps we can reload the page and get the info from that, or should we
let it go?
- should there be a _Quit in the file menu?

stealing bug off blake as well.

also, i'll see if I can fix the view source title, which for some odd reason is
not working (not related to my patch).

Comment 16

[Doron Rosenberg (IBM)]

Attachment: patch (needs feedback)

Comment 17

[Doron Rosenberg (IBM)]

ok, i attached the initial patch, need feedback (code wise and feature wise).

File                    Edit         Search              View                Help

New Navigator Window    Copy         Find in This Page   Text Size (x%)>      
                                                         --------------
New                     Select All   Find Again          Page Info

Open Web Location                                        Character Coding>      
---------------------
Close

Save AS
---------------------
Edit Page
---------------------
Page Setup

Print

Comment 18

[Sean Richardson]

Doron, Items look good, item order looks sensible. Two concerns:
1. The View menu should be between the Edit and Search menu. Quick cut-n-paste.
2. On the Edit menu the Undo, Redo, Cut, Paste, and Delete items should appear, 
   permanently greyed-out, along with the first two seperators, so that
   muscle memory for the positions of the Copy and Select All items is 
   preserved.
Beyond that, go for it. 

Comment 19

[timeless]

sean is only half right about (2), but that's a platform nightmare, someone 
(probably me) should write a bit of class foo that does the right thing (tm).

Comment 20

[Sean Richardson]

Timeless, details please.

If by "only half right" you mean the Edit menu is different on Macs, 
I'd suggest opening a new bug for a platform-specific-Mac version of
this bug. Matthew's comment of 2001-02-06 is persuasive, 
and given Paul Chen's comment of 2001-02-28 that disabling items is
different on Macs again, it looks like it's own task anyway.

Comment 21

[timeless]

erm. i'm tired, ignore me. wrt edit.
however, close does have platform locality features. as does the quit/exit name 
:)

Comment 22

[Doron Rosenberg (IBM)]

Attachment: more fun

Comment 23

[Doron Rosenberg (IBM)]

ok, all is fixed. 

timeless: you really want a view:reload?

Comment 24

[timeless]

+  <script type="text/javascript" src 
i really want application/x-javascript

and unless someone says no to reload i'd like it.

:) keep up the good work

Comment 25

[ajbu]

Building on W2K:
After applying this patch I get the error when trying to view the source:
XML Paring error: undefined entity Line Number 57, Column 77:
<menuitem observer="cmd_newNavigator" key="key_newNavigator" 
value="&newNavigatorCmd.label;" accesskey="&newNavigatorCmd.accesskey;"/>

It seems that newNavigatorCmd is defined in utilityOverlay.dtd.
After adding utilityOverlay.dtd it works.

+<!ENTITY % utilityDTD SYSTEM "chrome://communicator/locale/utilityOverlay.dtd" 
>
+%utilityDTD;

Hmm  View/Line numbers and Search/line-number menus would be handy :)

Comment 26

[Doron Rosenberg (IBM)]

for some reason this worked on my computer, and I found out why. I must have
forgotten it, but I had modifed the viewsource dtd file to have some stuff early on.

stupid mistake, attaching new patch.

Comment 27

[Doron Rosenberg (IBM)]

Attachment: new patch

Comment 28

[Doron Rosenberg (IBM)]

ok, it was not a missing dtd, just the wrong entity name I was using. This
should work now, I killed my local dtd and should work now.

As for the reload, that would require the inclusion of yet another dtd just for
"reload", not sure if it is worth it.

Comment 29

[Doron Rosenberg (IBM)]

Attachment: new patch reflecting blake's xul changes and mkaply's dtd addition

Comment 30

[Doron Rosenberg (IBM)]

ok, new and hopefully final patch reflecting changes to xul by blake and
mkaply's brand.dtd insertion in another bug.

Any comments on the code? if not, any r=?

Comment 31

[André Dahlqvist]

Just a small request: would it be possible to add a menu entry where you could
toggle wether syntax highlighting is done in the view source window? I know this
can be done through a pref, but it would be nice to have it in this window.

Comment 32

[Doron Rosenberg (IBM)]

syntax highlighting is now a pref and on by defualt. there is a bug to speed it up.

I have a patch that yet again brings it to the level of blake's xul changes. I
am waiting for my search backwards function to work before attaching a patch

Comment 33

[Doron Rosenberg (IBM)]

page info: it does nothing, should I remove it? Making it work is almost (if not
fully) impossible.

also, should save page use the original file's name? It does so at the moment
and seems the easiest...

other than that, patch is ready at http://www.nexgenmedia.net/viewsourcepatch.txt

Comment 34

[sairuh (rarely reading bugmail)]

cc'ing daniel to see if he has an opinion on Page Info access here. as an
alternative, how about loading [document] Properties instead [like timeless'
2001-03-06 08:05 comment]? unless that's difficult/not worth doing?

Comment 35

[Doron Rosenberg (IBM)]

blake: since you did a lot of xul cleanup, can you comment on the code.

I can save the contentAreaUtils.js if i copy the savePage() code into
viewsource.js if you want. Note that for contextmenus, nscontextmenu.js will
have to be included in the near future...

For the overlays, well I think all 4 are a must.

Comment 36

[Daniel Brooks [:db48x]]

I say disable it [page info menu item] for now, I'll prduce a patch that fixes
both ends at some point. You may file a bug on me if you like.

Basically I think what I'll do is change the page info window so that you pass
in an object with various properties. I'll have to change everywhere it's opened
from, but I suppose it'll be worth it.

Yay, more work for me. :)

db48x

Comment 37

[Daniel Brooks [:db48x]]

Oh, and you can file a bug on me regarding that if you like.

Comment 38

[Alexey Chernyak]

as requested in bug 22022 Word Wrap is something that's really needed in View
Source window.
Is it possible to implement View -> Word Wrap in the menu?

Comment 39

[Matthew T (active 1999-2002)]

No, Alexey, it is not possible to make features magically become implemented 
just by including items for them in the menus. Sorry.

Comment 40

[Alexey Chernyak]

Matthew, any menu is made for a feature.
When you make a menu, you also make the features this menu performs.
Seems quite logical to me.
If this bug is about making a dummy menu, which does nothing but sits there,
then yes I guess I asked my question in a wrong place.

Thank you for explaining to me relation between features and magic.
You opened my eyes. Appreciated. I been thinking about it last few nights.
I'm really shocked. I always thought Santa and Elves were making Mozilla. :o)

Comment 41

[sairuh (rarely reading bugmail)]

filed bug 76250 per daniel's 2001-04-10 13:59 and 14:00 comments.

Comment 42

[Doron Rosenberg (IBM)]

if the backend exists and works, then the front end can be added.  I have an
idea how the backend might be done, but it depends on the viewsource rewrite
landing and the syntax highlighting speedup landing.

Comment 43

[Doron Rosenberg (IBM)]

0.9.1, won't make it in today unless blake reviews today (i want him to do it
since he did a lot of xul changes, unless he thinks someone else can do it too).

New patch at http://www.nexgenmedia.net/viewsourcepatch.txt, comments out the
pageinfo stuff.

Comment 44

[Daniel Brooks [:db48x]]

you can actually reenable the page info menu item if bug 70632 gets checked in.
Not my recommended fix, but apparently someone else out there wanted the same
kind of thing.

db48x

Comment 45

[Daniel Brooks [:db48x]]

err, bug 63026 I mean

Comment 46

[Daniel Brooks [:db48x]]

doh, see bug 70362 :P

Comment 47

[Doron Rosenberg (IBM)]

new patch will be added in the next few days that will eradicate the need of
navigatorOverlay.xul and navigator.js, thus improving performance

Comment 48

[Doron Rosenberg (IBM)]

Attachment: Yet Another Bloody Patch

Comment 49

[Doron Rosenberg (IBM)]

added a new scary patch.  Removes dependencies from navigatorOverlay ans
navigator.js, as jag suggested.

any comments? or even r=?

Comment 50

[timeless]

is there a need to mark these files as NPL instead of MPL?

Comment 51

[Doron Rosenberg (IBM)]

i copied the ones from navigatorOverlay.xul and navigator.js...

Comment 52

[Doron Rosenberg (IBM)]

Attachment: mpl added

Comment 53

[Doron Rosenberg (IBM)]

i hope i got the mpl stuff correct, shoot me if not. Also removed some context
menu foo, which is another bug.

cc: jag for comments

Comment 54

[timeless]

*Bang*
+   - The Initial Developer of the Original Code is Netscape
+   - Communications Corporation.  Portions created by Netscape are
+   - Copyright (C) 2000 Netscape Communications Corporation.  All
+   - Rights Reserved.
!Bang!

Comment 55

[Doron Rosenberg (IBM)]

bah, netscape wrote the original code ;) It's like that on all other files.

Comment 56

[Doron Rosenberg (IBM)]

cc:ing ben goodger, who on irc said he thinks it best to overlay navigator.js
and navigatorOverlay as duplicating this much can cause problems.  I'll attach a
patch which does this and let peopel choose which one to check in. Since this
blocks a few bugs and i think some mac bugs too, would be nice for 0.9.1

Comment 57

[Doron Rosenberg (IBM)]

Attachment: latest patch - overlays navOverlay and includes navigator.js

Comment 58

[Doron Rosenberg (IBM)]

i just noticed that the last file has a "no newline at end" comment, will fix
that after i get some more feedback. anyone, or even r/sr? 

This patch includes navigator.js and navigatorOverlay.xul.  It modifies
openLocation.js to make viewsource be able to load locations directly. 

timeless: mpl is killing me, suggestions welcome.

Comment 59

[Doron Rosenberg (IBM)]

to explain the 
"if
(window.opener.document.getElementById("main-window").getAttribute("windowtype")
== "navigator:view-source"){"

Basically, to make "open location" work in viewsource, need to add a
view-source: infront of it. Currently i get the windowtype, but this might not
be the best way. Any other ideas?

Comment 60

[Doron Rosenberg (IBM)]

ok, i have yet another patch, which makes the open location less costly.  I also
added a dropdown option to the openlocation.xul called "Current ViewSource
Window" which is hidden unless viewsource is the caller.  I bet the string needs
to be refined, other than that, I have a final patch so I can get rid of this

Comment 61

[Doron Rosenberg (IBM)]

Attachment: patch

Comment 62

[Doron Rosenberg (IBM)]

new patch, I removed the open web location menuitem because making it work in
viewsource is complex and will be done after this lands.

i like talking to myself, i do

Comment 63

[Ben Goodger (use ben at mozilla dot org for email)]

OK. This seems reasonable. 
sr=[e-mail]

Comment 64

[jag (Peter Annema)]

I can see this is a copy&paste thing (stuff like ``id = "...."'' and commented
out lines which don't really apply to the view source window), but all in all it
looks okay. I really don't like us including all of navigator.js there, but I
understand one of the next steps is splitting that up so only the stuff that's
needed is shared. r=jag

Comment 65

[Doron Rosenberg (IBM)]

jag: the two commented out lines are:

1) print preview, which does not work. I kept that in so when it does, it can be
uncommented. 

2) page info - there is a bug filed about making it work in viewsource (76250),
so I kept it in as a reminder i guess.

The id's are the same for the overlay. But yes, this is a temp thing

Comment 66

[Asa Dotzler [:asa]]

a= [e-mail] for checkin to the trunk.
(on behalf of drivers)

Comment 67

[Doron Rosenberg (IBM)]

thanks!

can anyone please check this in?

Comment 68

[Doron Rosenberg (IBM)]

fixed! hwaara checked it in

Comment 69

[sairuh (rarely reading bugmail)]

yay! vrfy fixed using 2001.06.19.08 comm bits on linux and mac, and
2001.06.18.09 comm bits on winnt.

Comment 70

[Matthew T (active 1999-2002)]

Filed bug 87514, `Lots of menus/menu items disappear when a source window has
focus'.