XUL error report in main window - <key id="addBookmarkKb key="&addCurPageCmd.commandkey;"/>

RESOLVED FIXED

Status

SeaMonkey
Build Config
--
major
RESOLVED FIXED
12 years ago
12 years ago

People

(Reporter: Sergei Dolgov, Unassigned)

Tracking

({fixed1.8})

Trunk
x86
BeOS
fixed1.8

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 3 obsolete attachments)

(Reporter)

Description

12 years ago
Mozilla Suite 1.8b2 for BeOS (built either from latest source tarball or from cvs.

<key id="addBookmarkKb key="&addCurPageCmd.commandkey;"/> error message at
bottom of main window
http://beos.spb.ru/mozilla/BeZilla_DTD_OVerlay_Bug.png

deleting old profile don't help.

that code appears in
xpfe/browser/resources/content/win/platformNavigationBindings.xul
and maybe in 
xpfe/browser/resources/content/mac/platformNavigationBindings.xul

I suspect that we are using (should use) Unix bindings instead, but unsure, what
happened now, as before all worked OK.

Also wondering about proper component to assign this bug
(Reporter)

Comment 1

12 years ago
JavaScript console output:
----------------------------------
Error: undefined entity
Source File: chrome://navigator/content/platformNavigationBindings.xul
Line: 26, Column: 5
Source Code:
    <key id="addBookmarkKb" key="&addCurPageCmd.commandkey;"
command="Browser:AddBookmark" modifiers="accel,shift"/>
-----------------------------------
(In reply to comment #0)
> http://beos.spb.ru/mozilla/BeZilla_DTD_OVerlay_Bug.png

This bug looks much like bug 298675,

> Also wondering about proper component to assign this bug

then it could be an Installer issue.

*****

<http://lxr.mozilla.org/mozilla/search?string=addCurPageCmd.commandkey>
{{
addCurPageCmd.commandkey

/xpfe/browser/resources/content/mac/platformNavigationBindings.xul, line 20 --
<key id="addBookmarkKb" key="&addCurPageCmd.commandkey;"
command="Browser:AddBookmark" modifiers="accel,shift"/>
/xpfe/browser/resources/content/win/platformNavigationBindings.xul, line 26 --
<key id="addBookmarkKb" key="&addCurPageCmd.commandkey;"
command="Browser:AddBookmark" modifiers="accel,shift"/>

/xpfe/browser/resources/locale/en-US/mac/platformNavigationBindings.dtd, line 7
-- <!ENTITY addCurPageCmd.commandkey "d">
/xpfe/browser/resources/locale/en-US/win/platformNavigationBindings.dtd, line 5
-- <!ENTITY addCurPageCmd.commandkey "d">
}}

Or your/BeOS build could be missing the .dtd file, more like bug 298769 !?
Severity: normal → major
Version: unspecified → Trunk
(In reply to comment #0)
> I suspect that we are using (should use) Unix bindings instead, but unsure, what
> happened now, as before all worked OK.

Or this is the true issue
<http://lxr.mozilla.org/mozilla/find?string=platformNavigationBindings.xul>
{{
/xpfe/browser/resources/content/mac/platformNavigationBindings.xul
/xpfe/browser/resources/content/unix/platformNavigationBindings.xul
/xpfe/browser/resources/content/win/platformNavigationBindings.xul
}}

But I can't tell, since I know nothing about BeOS.
> This bug looks much like bug 298675,

why do you say that? the message is clearly different.

Comment 5

12 years ago
Hmm, do you have a platformNavigationBindings.xul without that entity in your
old builds?

Comment 6

12 years ago
I think bug 255036 added platform specific #ifdefs for xpfe/browser/jar.mn that
has windows as the fallback option. If I'm not mistaken the fallback was UNIX in
some Makefile, which makes for quite an interesting combination.

Comment 7

12 years ago
The patch there looks broken for all platforms that are not Mac, Unix or
Windows. It uses Unix as a fallback in all the makefiles and Windows in the two
jar.mn so there is mix of a Unix/Windows built. If that's what causing the
XUL-error I'm not sure, but we are using this file on BeOS
/xpfe/browser/resources/content/win/platformNavigationBindings.xul with that
change while we didn't have one before.

Comment 8

12 years ago
From what I see, the fallback is Unix all over the code (even in installer
platform detection etc.), so it clearly should also be in the jar.nm files. Only
OS/2 should share files with Windows, all others (that aren't clearly
win/mac/unix) fall into Unix category in our source code.

Comment 9

12 years ago
My fault - I thought Windows and OS/2 were the only non-Unices...
Assignee: p_ch → nobody
Component: Bookmarks → Build Config
QA Contact: bookmarks → build-config

Comment 10

12 years ago
FWIW,
http://lxr.mozilla.org/mozilla/source/xpfe/communicator/jar.mn#22
has the same problem as
http://lxr.mozilla.org/mozilla/source/xpfe/browser/jar.mn#33
and should be patched in the same way...
(In reply to comment #4)
> > This bug looks much like bug 298675,
> 
> why do you say that? the message is clearly different.

Yes, I did not meant they were duplicates;
I meant they show up the same way, and the fixes might be of the same kind.
('look' was the key word ;->)
Blocks: 255036
(Reporter)

Comment 12

12 years ago
So, what is proper way to fix those two jar.mn-s?
1)Add BEOS case and leave all remaining as is (Win as default/fallback).
2)Add BeOS, add Win and OS2, remove UNIX, makeing last default.
3)Don't add BeOS, but Add Win and OS2 and make Unix default.

Comment 13

12 years ago
(In reply to comment #12)
> 3)Don't add BeOS, but Add Win and OS2 and make Unix default.

That's the way to go for sure, as we're doing the same in multiple other cases,
and we should stick to the same way of things where possible.
(Reporter)

Comment 14

12 years ago
Need advise - how to activate and test changes in jar.mn-s properly?
Tried
1)make clean, make, make chrome in related folders. No luck.
2)same in xpfe folder - no luck
3) make distclean in moxilla/xpfe, configure in mozilla, make, make chrome in
xpfe - again in vain.

Interesting is that make for sure processes jar.mn - when i leave extra space
after #ifdef XP_UNIX, make reported about non-existing "XP_UNIX " identifier
(Reporter)

Comment 15

12 years ago
Problem looks more complex than expected.
No one of "make"s has any effect until i remove manually basic jar-s from
dist/bin/chrome. (so, in my case, maybe some bit different make target is
required, like 'make dist" or such).

But when done (unix as default in jar.mm-s, or corrected win bindings file),
effect is sad - menu toolbar is empty in best case, empty bug bar in XUL at
bottom of window - or, in worst case - Mozilla don't launch properly, only
half-empty windows with XUL bug report (or such) rises.

So, i suspect, we had use for BeZilla mix of unix and win defaults in XUL (but
maybe it was window everywhere) - and that worked before.

More investigation needed:(

Updated

12 years ago
Blocks: 275124

Updated

12 years ago
No longer blocks: 275124
(Reporter)

Comment 16

12 years ago
Got the reason, but need idea about better fix.
We use xpfe/browser/resources/content/win/platformNavigationBindings.xul (so MS
Windows one)
but mozilla/xpfe/browser/resources/locale/en-US/unix/platformNavigationBindings.dtd
(so for unix).
And, as you can guess now, there is:
<!ENTITY addCurPageCmd.commandkey "d"> in Windows *.dtd
but no such item in unix *.dtd

Adding mentioned entry in unix *.dtd fixes problem for BeOS build,
but will affect also all unix platfroms, which isn't our goal

Comment 17

12 years ago
i'd be fine w/ adding the item to the unix dtd, but should beos be using the 
win dtd?

i don't think adding a missing entity to the unix file should hurt unix, unless 
someone manages to include some other dtd that defines it. so i'd suggest doing 
that first. you can arrange to figure out the mess later...

Comment 18

12 years ago
(In reply to comment #16)
>Got the reason, but need idea about better fix.
As KaiRo already said, change those jar.mn files to use #ifdef XP_WIN & XP_OS2
(Reporter)

Comment 19

12 years ago
Neil, KaiRo,
actually i like to have that working cmd-D shortcut in BeOS too, for bookmarking.
And if it don't conflict with anything in "unix" - why not to simply add it to
*.dtd?

Comment 20

12 years ago
(In reply to comment #19)
>why not to simply add it to *.dtd?
To avoid confusion, platform .dtd files should match their .xul files.
(Reporter)

Comment 21

12 years ago
Created attachment 198527 [details] [diff] [review]
patch (diff -up6)

Moves ex-default lines to XP_WIN32 case, duplicates same to XP_OS2 case, and
moves ex XP_UNIX lines to default (last #else)
Does it for both communicator and browser

Anyone to look, review and commit?

Comment 22

12 years ago
Comment on attachment 198527 [details] [diff] [review]
patch (diff -up6)

If in any way possibe, there should be a single ifdef case for win32 and os2,
not two different ones. basically, the trend is correct though.
Attachment #198527 - Flags: review-
I think you misplaced an #else in the second file of the patch

unfortunately I don't think that the preprocessor supports something like #if
defined(FOO) || defined (BAR)
(Reporter)

Comment 24

12 years ago
Created attachment 198559 [details] [diff] [review]
patch

same as previous, but #else now at proper place,I hope (thanks biesi).
Yeah, i tried #if defined() || defined() like contructions, but it didn't work.

pp is really old-fashioned
Attachment #198527 - Attachment is obsolete: true
(Reporter)

Comment 25

12 years ago
Created attachment 198622 [details] [diff] [review]
patch - 3rd attempts

taking in account Neil's notice about mistakenly mixed lines from communicator
and browser
Attachment #198559 - Attachment is obsolete: true
(Reporter)

Comment 26

12 years ago
Created attachment 198626 [details] [diff] [review]
patch

restring empty line before en-US.jar:  header
(Reporter)

Updated

12 years ago
Attachment #198622 - Attachment is obsolete: true
(Reporter)

Comment 27

12 years ago
2005-10-08-05-trunk - patch DOESN'T HELP anymore.

bookmark naviagtion seems placed now in platformNavigationBindings.xul
and there wasn't such file mentioned in mn.jar-s, at least when i did the patch.

It seems development moves faster than patches can be reviewed.
That's bad idea - add changes ignoring obvious existing bugs:(((
(Reporter)

Comment 28

12 years ago
Comment on attachment 198626 [details] [diff] [review]
patch

ignore previous comment, just full chrome remove and make chrome in root folder
was required.
Tired of double building.

asking review to get this fix in tree at last.
Attachment #198626 - Flags: review?(cbiesinger)
Attachment #198626 - Flags: review?(cbiesinger) → review+
(Reporter)

Comment 29

12 years ago
Comment on attachment 198626 [details] [diff] [review]
patch

asking Neil for sr (and commit)
Attachment #198626 - Flags: superreview?(neil.parkwaycc.co.uk)
patch works fine on linux.
also works on win32.

Updated

12 years ago
Attachment #198626 - Flags: superreview?(neil.parkwaycc.co.uk) → superreview+

Comment 32

12 years ago
Fix checked in to the trunk.
Status: NEW → RESOLVED
Last Resolved: 12 years ago
Resolution: --- → FIXED
(Reporter)

Comment 33

12 years ago
wondering if there is sence to land it also in older 1.8 Mozilla Suite branch,
if it exists.

In theory this bug may affect other than BeOS systems

Comment 34

12 years ago
(In reply to comment #33)
> wondering if there is sence to land it also in older 1.8 Mozilla Suite branch,
> if it exists.

Surely it makes sense, as we plan to release SeaMonkey 1.0 from that branch.
Just request approval on the patch...
(Reporter)

Comment 35

12 years ago
Comment on attachment 198626 [details] [diff] [review]
patch

asking approval for SeaMonkey 1.0 branch
Attachment #198626 - Flags: approval1.8rc1?

Updated

12 years ago
Attachment #198626 - Flags: approval1.8rc1? → approval1.8rc1+

Updated

12 years ago
Keywords: fixed1.8
You need to log in before you can comment on or make changes to this bug.