159798 - File/Page Setup menu item does nothing from JavaScript Console

Status: RESOLVED FIXED

(Core Graveyard :: Error Console, defect)

Description

[Jean-Pierre Melkonian]

From Bugzilla Helper:
User-Agent: Mozilla/5.0 (Macintosh; U; PPC; en-US; rv:1.1b) Gecko/20020723
BuildID:    2002072308

The print setup in file menu of Javasript console
don't work

Reproducible: Always
Steps to Reproduce:
1.Open Javascript console
2.Open FileMenu-PageSetup
3.

Actual Results:  Nothing

Expected Results:  Page setup Dialog

Comment 1

[Wesha]

Mac specific issue?

2002072707/win98 JS console menu:

File
  Close
  -----
  Exit


No print stuff.

Comment 2

[Anthony DeRobertis]

There is no print stuff under Linux (2002072204) either.

Comment 3

[Jean-Pierre Melkonian]

On My Mac
File
  Close
  Page Setup...
  -----
  Exit

I mac User Please to confirm

Comment 4

[Greg K.]

Confirmed using Mac/2002080208/9.2.2.

Comment 5

[Pascal Chevrel:pascalc]

Could a MacOSX user tell me if this option still appears in the javascript console ?

Comment 6

[Bruce Davidson]

I can confirm that on MacOS X we still have a duff Page Setup menu item in the
JavaScript console for Mozilla 1.5b.

I'll have a go at a patch to remove it this weekend.

Comment 7

[Gervase Markham [:gerv]]

Bruce: note related bug 163641.

Gerv

Comment 8

[Bruce Davidson]

Attachment: Patch to remove Page Setup menu item from global overlay

The problem is that the page setup menu is added in the global overlay for
communicator. It is also defined by the particular overlays that require it
(e.g. mailnews, navigator). Removing it from the global overlay doesn't affect
it there, but removes it from JavaScript Console (and also bookmark manager,
see bug 163641).

This patch will currently cause Composer to lose its Page Setup menu item
because it relied on the global one. Not requesting review until this is
addressed.

Comment 9

[Bruce Davidson]

Attachment: Patch v2 to remove Page Setup menu item from global overlay

Revised patch that adds the page setup command into the editorOverlay so that
composer retains its Page Setup menu item following deletion of it from the
global overlay. Requesting review.

Comment 10

[Bruce Davidson]

Taking bug.

Comment 11

[Gervase Markham [:gerv]]

Bruce: you need to be chasing reviews on this patch if we are to get it into 1.5 :-)

Gerv

Comment 12

[Bruce Davidson]

Attachment: CVS patch

CVS patch against the trunk.

Comment 13

[:deanis74]

Comment on attachment 131196 [details] [diff] [review]
CVS patch

Does this maintain File > Page Setup in the browser?

Comment 14

[Bruce Davidson]

Yes. I have checked (on OS X 10.2.6):

Page Setup present and working:
Browser
Mail
Mail compose
Composer

Page Setup removed:
Browser - manage bookmarks
Browser - JavaScript console

Comment 15

[[e-mail]]

I would like to point that the editor module owner recently reviewed a similar
patch to Composer, attachment 131557 [details] [diff] [review], which did not rename the entity. I suggest
you are unlikely to receive module owner approval for the rename, as that would
make it inconsistent with the rest of editorOverlay.dtd, but you are welcome to
seek moa, or alternatively you could wait until the Composer portion of the
patch gains superreview and only check in the mac portion of your patch.

Comment 16

[Bruce Davidson]

To my mind it seems wrong to have two entities with different roots for the
access key and the label of the same control. As the access key isn't in use
until this patch it seems safe to rename it. However if the editor team would
prefer I'll respin the patch, without renaming the access key entity in editor,
this evening.

Comment 17

[[e-mail]]

Note that attachment 131557 [details] [diff] [review] has now been checked into the trunk so pleae make
sure that your patch won't conflict with it.

Comment 18

[Bruce Davidson]

Comment on attachment 131196 [details] [diff] [review]
CVS patch

As Neil commented this patch needs to be adapted following an earlier checkin
on editor. Will do this early next week.

Comment 19

[Frankie]

Bruce, was your patch ever checked in? Is this fixed?

If not, what is the relationship between this and bug 21296?

Comment 20

[Frankie]

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

Comment 21

[Bruce Davidson]

No, my patch was never checked in. Will have a go later this week at preparing a
revised patch that addresses the original concerns about renaming the entity.

Comment 22

[Stefan [:stefanh]]

Attachment: Updated version of Bruce Davidson's patch (Checked in)

The only thing needed now is to remove the menuitem in
mac/platformCommunicatorOverlay.xul.

Comment 23

[[e-mail]]

Comment on attachment 184263 [details] [diff] [review]
Updated version of Bruce Davidson's patch (Checked in)

This is the same as attachment 131143 [details] [diff] [review] isn't it?

Comment 24

[Stefan [:stefanh]]

Comment on attachment 184263 [details] [diff] [review]
Updated version of Bruce Davidson's patch (Checked in)

Low-risk patch for Seamonkey that fixes a long-time mac issue with a useless
"Page Setup..." menuitem in Bookmarks manager and JavaScript console.

Comment 25

[Stefan [:stefanh]]

(In reply to comment #23)
> (From update of attachment 184263 [details] [diff] [review] [edit])
> This is the same as attachment 131143 [details] [diff] [review] [edit] isn't it?
> 

ah, yes it is.

Comment 26

[Ian Neal]

Comment on attachment 184263 [details] [diff] [review]
Updated version of Bruce Davidson's patch (Checked in)

Checking in
xpfe/communicator/resources/content/mac/platformCommunicatorOverlay.xul;
new revision: 1.12; previous revision: 1.11
done

Comment 27

[Stefan [:stefanh]]

--> Fixed