Closed Bug 463179 Opened 13 years ago Closed 13 years ago

thunderbirdOverlay.xul has 2 |id="gloda-hello"|

Categories

(MailNews Core :: Backend, defect)

defect
Not set
normal

Tracking

(Not tracked)

VERIFIED FIXED
Thunderbird 3.0b1

People

(Reporter: sgautherie, Assigned: asuth)

References

()

Details

Attachments

(2 files, 1 obsolete file)

{
*** 3913 INFO TEST-PASS | chrome://mochikit/content/chrome/suite/common/tests/chrome/test_idcheck.xul | check id: messenger.xul#gloda-hello
Double id='gloda-hello' detected in chrome://messenger/content/messenger.xul:
  Tree 0:
    menuitem gloda-hello [19]
    menupopup taskPopup [0]
    menu tasksMenu [5]
    menubar mail-menubar [0]
    toolbox mail-toolbox [28]
    window messengerWindow [31]

    #document undefined [0]
  Tree 1:
    menuitem gloda-hello [20]
    menupopup taskPopup [0]
    menu tasksMenu [5]
    menubar mail-menubar [0]
    toolbox mail-toolbox [28]
    window messengerWindow [31]
    #document undefined [0]
*** 3914 ERROR TEST-UNEXPECTED-FAIL | chrome://mochikit/content/chrome/suite/common/tests/chrome/test_idcheck.xul | check id: messenger.xul#gloda-hello
}
Taken from
http://tinderbox.mozilla.org/showlog.cgi?log=SeaMonkey/1225865721.1225872220.4346.gz#err31
for example
Maybe this warning in the error console has the same reason?

> Warning: Warning: Duplicate resource declaration for 'gloda' ignored.
> Source File: file:///Volumes/Shredder/Shredder.app/Contents/MacOS/chrome/gloda.manifest
Line: 1
This is a quick fix.  Gloda should either not have any strings or should migrate them to thunderbird's strings proper.
Assignee: nobody → bugmail
Status: NEW → ASSIGNED
Attachment #346465 - Flags: review?(dmose)
Attachment #346465 - Flags: review?(dmose)
Comment on attachment 346465 [details] [diff] [review]
v1 resolve duplicate id while removing unused menu item

>-    <menuitem id="gloda-hello" label="&glodaIndexEverything.label;" 
>+    <menuitem id="gloda-index-everything" label="&glodaIndexEverything.label;" 
>               oncommand="gloda.onIndexEverythingCommand(event);"/>
>-    <menuitem id="gloda-hello" label="&glodaIndexAddressBook.label;" 
>-              oncommand="gloda.onIndexAddressBookCommand(event);"/>

If you do this, you need to remove the related "code" too:
http://mxr.mozilla.org/comm-central/search?string=gloda-hello&case=on&find=%2Fmailnews%2Fdb%2Fgloda%2F
http://mxr.mozilla.org/comm-central/search?string=glodaIndexAddressBook.label&find=%2Fmailnews%2Fdb%2Fgloda%2F
...
(In reply to comment #1)
> Maybe this warning in the error console has the same reason?

I doubt it: you should better file a separate bug, with steps to reproduce.
(In reply to comment #4)
> (In reply to comment #1)
> > Maybe this warning in the error console has the same reason?
> 
> I doubt it: you should better file a separate bug, with steps to reproduce.

OK, I opened Bug 463278 for this.
Comment on attachment 346465 [details] [diff] [review]
v1 resolve duplicate id while removing unused menu item

This never got UI-review in the first place; can you instead create a patch that simply stops overlaying entirely?
Attachment #346465 - Flags: review-
(In reply to comment #6)
> (From update of attachment 346465 [details] [diff] [review])
> This never got UI-review in the first place; can you instead create a patch
> that simply stops overlaying entirely?

The overlay is currently the means by which we compel gloda to be loaded.  Should we avoid having gloda load unless there is actual code that tries to load it (ex: exptoolbar's instantiation of glodacomplete), or should we create a JS component that explicitly loads gloda, at least if the gloda indexing pref is enabled?
Hmmm, good point.  I guess we should leave it overlaying so that we test whether indexing causes performance problems for people who have it enabled.  Maybe instead just get rid of the menu items from the overlay?
Attachment #346465 - Attachment is obsolete: true
Attachment #347034 - Flags: review?(dmose)
Target Milestone: --- → Thunderbird 3.0b1
Whiteboard: [needs review dmose]
Flags: blocking-thunderbird3+
Duplicate of this bug: 463606
Attachment #347034 - Flags: review?(dmose) → review+
Comment on attachment 347034 [details] [diff] [review]
v1 nuke the overlay down to compelling gloda to load

r=dmose
Checked in, changeset id: http://hg.mozilla.org/comm-central/rev/6910974be89b
Status: ASSIGNED → RESOLVED
Closed: 13 years ago
Resolution: --- → FIXED
Whiteboard: [needs review dmose]
V.Fixed, between
http://tinderbox.mozilla.org/showlog.cgi?log=SeaMonkey/1226516779.1226521004.9976.gz
Win2k3 comm-central dep unit test on 2008/11/12 11:06:19
and
http://tinderbox.mozilla.org/showlog.cgi?log=SeaMonkey/1226520617.1226524860.21208.gz
Win2k3 comm-central dep unit test on 2008/11/12 12:10:17
for example
Status: RESOLVED → VERIFIED
"... no chrome package registered for ..."
Attachment #348257 - Flags: superreview?(dmose)
Attachment #348257 - Flags: review?(dmose)
Attachment #348257 - Flags: superreview?(dmose)
Attachment #348257 - Flags: superreview+
Attachment #348257 - Flags: review?(dmose)
Attachment #348257 - Flags: review+
Comment on attachment 348257 [details] [diff] [review]
And your little doctype Toto, too

r+sr=dmose; ROFL at the patch description
You need to log in before you can comment on or make changes to this bug.