264102 - Chrome should use document.title instead of window.title

Status: RESOLVED FIXED

(Core :: DOM: Core & HTML, defect)

Description

[[e-mail]]

<Neil> jst: so, why did we have a title attribute on all window objects?
<jst> Neil: because someone screwed up and added it for no reason

Note that window.title is only available on chrome windows, and it has its own
implementation, while document.title is correctly loaded from the XUL.

Comment 1

[[e-mail]]

Attachment: Make deprecated window.title shadow document.title

Comment 2

[Johnny Stenback (:jst)]

Comment on attachment 162334 [details] [diff] [review]
Make deprecated window.title shadow document.title

sr=jst

Comment 3

[Ian Neal]

Attachment: xpfe fixes v0.1

Comment 4

[Ian Neal]

Attachment: mailnews fixes v0.1

Comment 5

[Ian Neal]

Attachment: toolkit fixes v0.1

Comment 6

[Ian Neal]

Attachment: mail fixes v0.1

Comment 7

[Ian Neal]

Attachment: editor fixes v0.1

Comment 8

[Ian Neal]

Attachment: p3p fixes v0.1

Could not find any sites to test this patch against

Comment 9

[Ian Neal]

Attachment: sroaming fixes v0.1

Not tested as I do not use sroaming

Comment 10

[Ian Neal]

Attachment: venkman fixes v0.1

Comment 11

[Ian Neal]

Attachment: xmlterm fixes v0.1

Comment 12

[Robert Ginda]

Comment on attachment 162926 [details] [diff] [review]
venkman fixes v0.1

r=rginda

Comment 13

[Daniel Glazman (:glazou) (not active in Mozilla any more)]

Comment on attachment 162922 [details] [diff] [review]
editor fixes v0.1

rs=[e-mail]

Comment 14

[[e-mail]]

Comment on attachment 162923 [details] [diff] [review]
p3p fixes v0.1

It looks like you're setting the title of the wrong document.

Comment 15

[Vladimir Vukicevic [:vlad] [:vladv] (needinfo me, slow to respond)]

Comment on attachment 162909 [details] [diff] [review]
toolkit fixes v0.1

r=vladimir

Comment 16

[[e-mail]]

Comment on attachment 162905 [details] [diff] [review]
mailnews fixes v0.1

Note that sendProgress.js is already covered by bug 260217.

Comment 17

[Ian Neal]

So the editor and mailnews patches in here will bit rot you?

Comment 18

[[e-mail]]

Well, you could always forget to check those files in ;-)

Comment 19

[Ian Neal]

Attachment: p3p fixes v0.2

Comment 20

[[e-mail]]

Comment on attachment 163123 [details] [diff] [review]
p3p fixes v0.2

Sorry, but this.title and window.title are synonyms at this point in the code.

Comment 21

[Ian Neal]

Attachment: p3p fixes v0.3

Comment 22

[[e-mail]]

Comment on attachment 163173 [details] [diff] [review]
p3p fixes v0.3

I don't think you need the XUL change though, it seems to work fine without it.

Comment 23

[Ian Neal]

Attachment: p3p fixes v0.3a

Comment 24

[Ian Neal]

Comment on attachment 163206 [details] [diff] [review]
p3p fixes v0.3a

Removed .xul changes, as not needed, carring forward r= and requesting sr=

Comment 25

[Ian Neal]

Checking in venkman fixes:
Checking in venkman-bpprops.js;
/cvsroot/mozilla/extensions/venkman/resources/content/venkman-bpprops.js,v  <--
 venkman-bpprops.js
new revision: 1.4; previous revision: 1.3
done

Comment 26

[Ian Neal]

Checking in sroaming fixes:
Checking in progressDialog.js;
/cvsroot/mozilla/extensions/sroaming/resources/content/transfer/progressDialog.js,v
 <--  progressDialog.js
new revision: 1.5; previous revision: 1.4
done

Comment 27

[Ian Neal]

Checking in xpfe fixes:
Checking in components/filepicker/res/content/filepicker.js;
/cvsroot/mozilla/xpfe/components/filepicker/res/content/filepicker.js,v  <-- 
filepicker.js
new revision: 1.91; previous revision: 1.90
done
Checking in global/resources/content/bindings/tabbrowser.xml;
/cvsroot/mozilla/xpfe/global/resources/content/bindings/tabbrowser.xml,v  <-- 
tabbrowser.xml
new revision: 1.104; previous revision: 1.103
done
Checking in global/resources/content/commonDialog.js;
/cvsroot/mozilla/xpfe/global/resources/content/commonDialog.js,v  <-- 
commonDialog.js
new revision: 1.55; previous revision: 1.54
done
Checking in global/resources/content/selectDialog.js;
/cvsroot/mozilla/xpfe/global/resources/content/selectDialog.js,v  <-- 
selectDialog.js
new revision: 1.21; previous revision: 1.20
done

Comment 28

[Ian Neal]

Checking in xmlterm fixes:
Checking in XMLTermChrome.js;
/cvsroot/mozilla/extensions/xmlterm/ui/content/XMLTermChrome.js,v  <-- 
XMLTermChrome.js
new revision: 1.8; previous revision: 1.7
done

Comment 29

[Ian Neal]

Checking in toolkit fixes:
Checking in mozapps/downloads/content/downloads.js;
/cvsroot/mozilla/toolkit/mozapps/downloads/content/downloads.js,v  <--  downloads.js
new revision: 1.31; previous revision: 1.30
done
Checking in content/widgets/tabbrowser.xml;
/cvsroot/mozilla/toolkit/content/widgets/tabbrowser.xml,v  <--  tabbrowser.xml
new revision: 1.50; previous revision: 1.49
done
Checking in content/selectDialog.js;
/cvsroot/mozilla/toolkit/content/selectDialog.js,v  <--  selectDialog.js
new revision: 1.2; previous revision: 1.1
done
Checking in content/commonDialog.js;
/cvsroot/mozilla/toolkit/content/commonDialog.js,v  <--  commonDialog.js
new revision: 1.3; previous revision: 1.2
done
Checking in components/filepicker/content/filepicker.js;
/cvsroot/mozilla/toolkit/components/filepicker/content/filepicker.js,v  <-- 
filepicker.js
new revision: 1.9; previous revision: 1.8
done

Comment 30

[Ian Neal]

Checked in mailnews fixes (apart from sendProgress.js which will be in fix to
bug 260217)

Comment 31

[[e-mail]]

Checked in the patch to send window.title to document.title with a warning.

Comment 32

[[e-mail]]

Comment on attachment 162922 [details] [diff] [review]
editor fixes v0.1

sr=me with EdColorPicker.js caveat.

Comment 33

[Ian Neal]

Comment on attachment 162922 [details] [diff] [review]
editor fixes v0.1

Checked in editor fixes without EdColorPicker.js changes as these will be fixed
in the patch to bug 260217

Comment 34

[Mike Kaply [:mkaply]]

Comment on attachment 163206 [details] [diff] [review]
p3p fixes v0.3a

I'm not an sr

Comment 35

[Ian Neal]

Comment on attachment 163206 [details] [diff] [review]
p3p fixes v0.3a

Checking in p3pSummary.js;
/cvsroot/mozilla/extensions/p3p/resources/content/p3pSummary.js,v  <-- 
p3pSummary.js
new revision: 1.7; previous revision: 1.6
done

Comment 36

[Ian Neal]

Comment on attachment 162920 [details] [diff] [review]
mail fixes v0.1

Checking in base/content/commandglue.js;
/cvsroot/mozilla/mail/base/content/commandglue.js,v  <--  commandglue.js
new revision: 1.42; previous revision: 1.41
done
Checking in components/addrbook/content/abAddressBookNameDialog.js;
/cvsroot/mozilla/mail/components/addrbook/content/abAddressBookNameDialog.js,v 
<--  abAddressBookNameDialog.js
new revision: 1.3; previous revision: 1.2
done
Checking in components/addrbook/content/abCardOverlay.js;
/cvsroot/mozilla/mail/components/addrbook/content/abCardOverlay.js,v  <-- 
abCardOverlay.js
new revision: 1.6; previous revision: 1.5
done
Checking in components/compose/content/MsgComposeCommands.js;
/cvsroot/mozilla/mail/components/compose/content/MsgComposeCommands.js,v  <-- 
MsgComposeCommands.js
new revision: 1.55; previous revision: 1.54
done

Comment 37

[Zibi Braniecki [:zbraniecki][:gandalf]]

Attachment: various (inspector,pki and calendar)

last 3 places fixed.

Comment 38

[Giacomo Magnini]

I think there is still one place where window.title is used and it's in the
browser download progress window. Anybody would like to take care of that too?
Neil's patch didn't address this (Seamonkey build ID: 2004111704).

Comment 39

[Ian Neal]

Comment on attachment 164333 [details] [diff] [review]
various (inspector,pki and calendar)

Could you request reviews for this patch please.

Comment 40

[Ian Neal]

Attachment: progressDlg fix v0.1

Comment 41

[[e-mail]]

Comment on attachment 166437 [details] [diff] [review]
progressDlg fix v0.1

That comment was misleading even before the patch and it's even more misleading
now; it would be nice if you could fix it using a meaningful form of words.

Comment 42

[[e-mail]]

Comment on attachment 164333 [details] [diff] [review]
various (inspector,pki and calendar)

>Index: ./extensions/inspector/resources/content/inspector.js
./ prefixes on your file names? Don't ask me to check this in for you.

>   var title = dialogParams.GetString(2);
>-  window.title = title;
>+  document.title = title;
Hardly worth a separate variable, might as well fold these two lines into one
while you're here.

>Index: ./calendar/resources/content/printDialog.js
I'll give you a chance to get a calendar review for this.

Comment 43

[Ian Neal]

Comment on attachment 166437 [details] [diff] [review]
progressDlg fix v0.1

Requesting a= for very simple, low risk fix

Will change comment in patch to
// Set dialog's title attribute.

Comment 44

[Zibi Braniecki [:zbraniecki][:gandalf]]

Attachment: various (inspector,pki)

per Comment 42.

Also removed calendar part because it's commented out for the moment.

Comment 45

[Ian Neal]

Comment on attachment 166437 [details] [diff] [review]
progressDlg fix v0.1

Checking in nsProgressDialog.js;
/cvsroot/mozilla/embedding/components/ui/progressDlg/nsProgressDialog.js,v  <--
 nsProgressDialog.js
new revision: 1.42; previous revision: 1.41
done

Comment 46

[Ian Neal]

Comment on attachment 166636 [details] [diff] [review]
various (inspector,pki)

Checking in security/manager/pki/resources/content/serverCertExpired.js;
/cvsroot/mozilla/security/manager/pki/resources/content/serverCertExpired.js,v 
<--  serverCertExpired.js
new revision: 1.14; previous revision: 1.13
done
Checking in extensions/inspector/resources/content/inspector.js;
/cvsroot/mozilla/extensions/inspector/resources/content/inspector.js,v	<-- 
inspector.js
new revision: 1.21; previous revision: 1.20
done

Comment 47

[[e-mail]]

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

Comment 48

[Ian Neal]

Certain parts will need to be relanded once aviary branch has landed on trunk

Comment 49

[Ian Neal]

toolkit fixes rechecked in after aviary landing

Comment 50

[Steffen Wilberg]

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

Comment 51

[Ian Neal]

Fixed

Comment 52

[Zibi Braniecki [:zbraniecki][:gandalf]]

Please, correct me if I'm wrong.
After aviary-landing:

http://lxr.mozilla.org/seamonkey/source/toolkit/components/filepicker/content/filepicker.js#75
http://lxr.mozilla.org/seamonkey/source/toolkit/content/commonDialog.js#102
http://lxr.mozilla.org/seamonkey/source/toolkit/content/selectDialog.js#79
http://lxr.mozilla.org/seamonkey/source/toolkit/mozapps/downloads/content/downloads.js#166
http://lxr.mozilla.org/seamonkey/source/toolkit/mozapps/downloads/content/downloads.js#413
http://lxr.mozilla.org/seamonkey/source/toolkit/mozapps/downloads/content/downloads.js#433
http://lxr.mozilla.org/seamonkey/source/toolkit/mozapps/downloads/content/downloads.js#446

There is also some code like:
var newW = window.open();
newW.title = "foo";

which is pretty hard to find for me...

Comment 53

[Steffen Wilberg]

It's fixed in cvs. lxr/seamonkey currently updates nightly.