122693 - Javascript errors in viewZoomOverlay.js

Status: VERIFIED FIXED

(SeaMonkey :: Help Documentation, defect)

Description

[Stephane Saux]

To reproduce:
Open javascript console.
From a browser window do: Help->Help and Support Center

You get:
Error: textZoomMenu has no properties
Source File: chrome://communicator/content/viewZoomOverlay.js
Line: 229

Since errors stop execution of the current js function this is potentially bad.

I have a patch crafted to make sure there are not errors. I have no ideas
whether that's the right thing to do.

Comment 1

[Stephane Saux]

Attachment: check that an object is valid before using its properties.

Comment 2

[jag (Peter Annema)]

Nope, going about it the wrong way. If you're including (directly or indirectly)
viewZoomOverlay.xul you promise to have a menu item to expose this
functionality. Don't break that promise.

Comment 3

[John Morrison]

Okay, so then 'help.xul' should not drag in the overlay for text zoom, if it is 
not providing a menu for it. Yes?

-> oeschger, and a one-line patch that squelches this error coming up

Comment 4

[John Morrison]

Attachment: remove viewZoomOverlay.xul from help.xul

Comment 5

[Ian Oeschger (gone)]

Accepting. I didn't realize it was a menu promise we were making. Pulling it out
of help.xul as part of the Big Patch in 67376. Should be checked in in the next
couple of days. 

Comment 6

[jag (Peter Annema)]

Alternatively you provide a few things and you have the text zoom for free:

- getMarkupDocumentViewer()

This you probably have already through browser.js

- the stringbundle:

<stringbundleset id="stringbundleset">
  <stringbundle id="bundle_viewZoom"/>
</stringbundleset>

- the key bindings:

<keyset id="viewZoomKeys"/>

- the commands:

<commandset id="viewZoomCommands"/>

- the menu item:

<menu id="menu_textZoom"/>

And that should do the trick.

I just realized no one updated the "requirements" section in viewZoomOverlay.js
(and it wouldn't hurt to move it to the xul file, either).

With some work I suspect this can be turned into a binding so we can move the
commands, keys and stringbundle into it and make this more/completely
self-contained, but for the moment it's just this list of requirements you'll
have to meet, sorry :-/

If you need any help with the above, don't hesitate to ask.

Comment 7

[Ian Oeschger (gone)]

Pulled out the viewZoomOverlay.js, but still need to add the zoom features in
the UI.

Comment 8

[Blake Ross]

oeschger, I notice ctrl+f doesn't work in help.

Error: browser is not defined
Source File: chrome://help/content/help.js
Line: 431

do you know when that broke?

Comment 9

[Ian Oeschger (gone)]

No idea, blake. Someone brought it to my attention last week for the first time.

Comment 10

[Daniel Wang]

is this bug still valid? If it is, someone please chg the summary

Comment 11

[Ian Oeschger (gone)]

moving stuff over to an outside-the-firewall email for the time being, looking
for people to pick these Help and doc bugs up for me.

Comment 12

[R.J. Keller]

viewZoomOverlay.xul was removed in revision 1.30 of help.xul, and what it looks
like from Ian's comments, resolving this as FIXED. Reopen if I am wrong.

Revision 1.30:
http://bonsai.mozilla.org/cvsview2.cgi?diff_mode=context&whitespace_mode=show&file=help.xul&branch=&root=/cvsroot&subdir=mozilla/extensions/help/resources/content&command=DIFF_FRAMESET&rev1=1.29&rev2=1.30

Comment 13

[Daniel Wang]

verified