41984 - Internet shortcut titles are empty

Status: RESOLVED FIXED

(Core :: DOM: Navigation, defect, P1)

Description

[Mike Pinkerton (not reading bugmail)]

when dragging the proxy icon to the desktop, we need to get the window title to 
put in the filename. navigatorDD.js line 321 accesses |window.title| and it is 
the empty string.

Stepping into nsGlobalWindow::GetProperty() shows we are getting a docShell and 
asking for its title, but nsDocShell::GetTitle() returns to us the empty string.

I put a breakpoint in nsDocShell::SetTitle() and reloaded the page. The docShell 
where the window is set is a different docShell from the one we access using the 
|window| object.

Comment 1

[Blake Ross]

"TEMP URL FILENAME" and the IE icon appear for me when dragging to the desktop

Comment 2

[Blake Ross]

guess that TEMP URL FILENAME text is just a placeholder --> 
http://lxr.mozilla.org/seamonkey/source/widget/src/windows/nsDataObj.cpp#418

Comment 3

[Mike Pinkerton (not reading bugmail)]

i haven't checked in the code that uses window.title yet, and in fact, it will be 
commented out when i do.

Comment 4

[locka]

The docshell only returns the string previously set through a call to SetTitle. 
Who is supposed to call this?

Comment 5

[don]

Adding "correctness" keyword.  We need this fixed for beta 3.

Comment 6

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

what's the status on this? 

vidur, I mentioned this bug to hyatt and he said you might know something about 
it? 

Comment 7

[Adam Lock]

It looks like the content docshell's title is being correctly set when the 
document loads, but the nsGlobalWindow.cpp code is asking for the wrong docshell 
for its title. The global window is asking the topmost chrome docshell 
for a title when it should be asking the primary content docshell. Working on a 
patch...

Comment 8

[Adam Lock]

Attachment: Global window gets and sets title from/to primary doc shell

Comment 9

[Adam Lock]

Review and approval please.

Once this patch is checked in, navigatorDD.js will needs to be fixed to remove 
the temporary hack around this problem that it contains.

Comment 10

[Johnny Stenback (:jst)]

Adam, I'm sorry to say this, but the title code in nsGlobalWindow.cpp changed
dramatically today, you're gonna haveto update (be carefull, there will be merge
conflicts) and create a new diff...

Comment 11

[Adam Lock]

Attachment: Here's the patch updated for the most recent changes. Review please

Comment 12

[Johnny Stenback (:jst)]

I say the patch looks good to me, r=jst

Comment 13

[Adam Lock]

Can I just clarify before I check in that this patch is doing the correct thing? 
I am mapping window.title property onto the primary content document's title 
property. If there is no primary content document, then window.title will return 
an empty string.

Comment 14

[Adam Lock]

Changes checked in.

Comment 15

[Joseph Elwell]

adamlock:backing out your change fixes bug 51569 (tested on win32) which is also
window titles are empty.

Comment 16

[Joseph Elwell]

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

Comment 17

[vidur (gone)]

window.title is, I believe, intended to refer to the top-level window's title. 
The fact that this maps to the top-level content document's title element is 
incidental. I would assume that window.title should be "Netscape" (or whatever 
our default window title is) if the top-level content document doesn't have a 
title. Similarly, modifying the window.title property should not change the 
top-level content document's title - it shoud merely change the value appearing 
in the title bar.

Comment 18

[Charles Manske]

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

Comment 19

[Phil Peterson]

PDT agrees P2

Comment 20

[Adam Lock]

Okay, I've looked at this afresh and the simple answer why window.title is empty 
is that no one ever calls to set it from the browser code. Naturally, that means 
when it comes to drag and drop time there is no string to create the new 
shortcut file with. When it is set (such as in the mail/news window), the 
window.title works as expected.

My previous mapped window.title mapped onto document.title but that made other 
people unhappy. Therefore I think the best solution is to change change drag and 
drop to use the document's title and not the window title. This is what it 
should do anyway. The patch follows.

The title, whether from the window or document still needs to be fixed up to 
remove illegal characters & empty strings but that's another bug.

Comment 21

[Adam Lock]

Attachment: Revised drag and drop code. Comments please

Comment 22

[Charles Manske]

This is very important! I can't believe this wasn't fixed before the 9/22
deadline! Use Composer - in mail or stand-alone. We have lots of dialogs, like
the color picker, and message dialogs that don't have titles because of this.
Raising priority -- we can't ship like this.

Comment 23

[Adam Lock]

Renaming this bug since it deals with dragging and dropping a shortcut onto the 
desktop. Bug 41984 deals with the empty title string in the mail / news window.

In the absence of any comments I will submit both patches to the reviewers 
list.

Comment 24

[vidur (gone)]

Bug 41984 is *this* bug. :-) Adam, what's the actual "title in mail/news" bug?

Comment 25

[Adam Lock]

Oops :) the mail / news bug is 50682.

Comment 26

[lchiang]

Is this really an "embed" bug (as noted in the keyword)?  I'm asking because I
don't want this bug to fall off the radar (since PDT is not looking at embed
keyword bugs).

Comment 27

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

patch looks fine. r||a=ben

Comment 28

[Michael La Guardia]

marking nsbeta3- as we don't need this to ship pr3.  The bug is already
nominated for rtm.

Comment 29

[Charles Manske]

Emphasizing problem by modifying summary

Comment 30

[Charles Manske]

I think there may be 2 bugs here. The patch by ben fixes the specific issue
of bookmark drag and drop, but not setting titles in "common dialogs" used
for messages.
Here is the JS we are using (from editor/base/nsEditorShell.cpp, 
nsEditorShell::Alert():
  NS_WITH_SERVICE(nsICommonDialogs, dialog, kCommonDialogsCID, &rv); 
  if (NS_SUCCEEDED(rv) && dialog)
  {
    if(!mContentWindow)
      return;
    nsCOMPtr<nsIDOMWindowInternal> cwP = do_QueryReferent(mContentWindow);
    if (!cwP) return;
    rv = dialog->Alert(cwP, aTitle.GetUnicode(), aMsg.GetUnicode());
  }
The title being passed to the "Alert" is not displaying.

Comment 31

[Charles Manske]

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

Comment 32

[Charles Manske]

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

Comment 33

[Adam Lock]

Renaming this bug back again. This bug deals with the internet shortcut code 
creating an empty filename because Navigator chrome never sets window.title in 
the first place.

Bug 50682 deals with the window.title value never being set on frame/dialog 
title bars.

Both bugs have approved & reviewed fixes but neither fix is going into beta 3. 
However, both bugs are nominated rtm and will be checked into the trunk as soon 
as it is permissable to do so.

Comment 34

[Adam Lock]

Fix checked into the trunk

Comment 35

[Blake Ross]

this needs to be marked rtm+ so it shows up on PDT radar.

Comment 36

[Adam Lock]

Adding rtm+ 

Comment 37

[Phil Peterson]

[rtm-]. Not crash/data loss, and there's an easy workaround to rename the
desktop shortcut.

Comment 38

[Adam Lock]

Marking as FIXED. It's in the trunk, won't be going in the branch.