215440 - Support titledefault attribute for specifying titles to use in lieu of document titles

Status: RESOLVED FIXED

(Core :: XUL, defect)

Description

[David Hyatt]

Necessary for XUL clients that don't wish to use the titlemodifier/separator stuff.

Comment 1

[David Hyatt]

Attachment: Implement titledefault

Comment 2

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

Comment on attachment 129373 [details] [diff] [review]
Implement titledefault

r=[e-mail]

Comment 3

[:deanis74]

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

Comment 4

[chris hofmann]

Comment on attachment 129373 [details] [diff] [review]
Implement titledefault

approved for 1.5b landing

Comment 5

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

If you're substituting something for the lack of a title, shouldn't you
distinguish documents without a TITLE element from documents with <TITLE></TITLE>?

Comment 6

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Comment on attachment 129373 [details] [diff] [review]
Implement titledefault

>+  if (!mTitlePreface.IsEmpty()) {
>+    // Title will be: "Preface: Doc Title - Mozilla"
>+    title.Assign(mTitlePreface);
>+    title.Append(docTitle);
>+  }
>+  else {
>+    // Title will be: "Doc Title - Mozilla"
>+    title = docTitle;
>+  }

Also, why bother with this if-else when the then-part would do the same thing
in all cases?  (Also, the comment is slightly misleading since it could be
taken to imply that a separator is inserted after the Preface.	Or did you
intend to do that, which would require the if-else?)

Comment 7

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Comment on attachment 129373 [details] [diff] [review]
Implement titledefault

...which would allow the entire function (other than the initial null-checks)
to be simplified to):

nsDependentString inTitle(aTitle);
nsAFlatString& title = PromiseFlatString(
  mTitlePreface +
  (inTitle.IsEmpty() ? mTitleDefault : inTitle) +
  mTitleSeparator +
  mWindowTitleModifier);

// XXX Don't need to fully qualify this once I remove
nsWebShellWindow::SetTitle
// return mXULWindow->SetTitle(title.get());
return mXULWindow->nsXULWindow::SetTitle(title.get());

Comment 8

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

But maybe you wouldn't want that to happen, since if there's no title, and the
XUL didn't provide a titledefault, you don't want to append the separator, do you?

Comment 9

[Tom Sommer]

You realize this also has major effects on Thunderbird?

I really do not approve of these changes. They should be applied to Mac ONLY...

The changes affects way to many applications for it to be be justified

Comment 10

[Brendan Eich [:brendan]]

Tom: mscott's in charge of thunderbird, and he sr'd the patch.  What other apps
are affected?  Does this patch do anything to SeaMonkey?  It shouldn't.

I'm more concerned that the code was crappy before and hyatt didn't fix it up
enough.  Hyatt, see dbaron's comments.

/be

Comment 11

[Tom Sommer]

I would think it affects all applications based on XUL. I don't know how many
that is?

Comment 12

[David Hyatt]

Tom, the C++ changes have zero effect on XUL applications.  All the C++ 
changes do is add an additional attribute called titledefault that can be used 
when document titles are empty (to avoid having a blank titlebar).  If you 
want to protest the removal of the titlemodifier and titleseparator attributes 
from the Firebird and Thunderbird XUL, go back to the other bug.  This bug is 
purely about adding a feature to XUL that is completely backwards-compatible 
with Seamonkey and other XUL apps.

dbaron, I just re-indented the code and removed conditionals (and added an 
init of docTitle to match the titleDefault).  The other code you're critiquing 
is the same as it was before.  If you want to rewrite it/clean it up, go for 
it, but that's not what this patch was about.

Also, I don't think there's any reason to distinguish between <title></title> 
and an untitled page.  The point is to avoid a blank titlebar (which looks 
terrible).

Comment 13

[David Hyatt]

dbaron, right.  If you have no doctitle (or titledefault), then you don't 
append the separator.

Comment 14

[David Hyatt]

Which makes what I did wrong.  Patch forthcoming.

Comment 15

[David Hyatt]

Attachment: Make sure the code is exactly the same as it was before except for the doctitle initialization

Comment 16

[Brendan Eich [:brendan]]

Comment on attachment 129388 [details] [diff] [review]
Make sure the code is exactly the same as it was before except for the doctitle initialization

r+sr+a=[e-mail], get it in for 1.5b.

/be

Comment 17

[David Hyatt]

Fixed.

Comment 18

[whorfin]

This is STILL broken.

It's fixed if you open a new Firebird WINDOW and set it to about:blank. 
However, if you open a new TAB, the title bar goes to completely blank.

Comment 19

[Bill Mason]

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

Comment 20

[Bill Mason]

Reopening in light of comment 18.

Comment 21

[whorfin]

BTW, sorry about opening Bug 215557.  I didn't know if this one would get
reopened or not... mea culpa.

Comment 22

[:deanis74]

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

Comment 23

[Dai Toyama]

Would someone please change the OS from MacOS X to Linux?  I see this problem
using Red Hat.

Sorry for having opened #215669.  Thanks for marking that duplicate, Dean.

Comment 24

[Tom Sommer]

It's not only Linux, it's ALL. Opening a new tab will make the "title" go away
in windows too...

Comment 25

[Bill Mason]

Hardware/OS All/All

Comment 26

[David Hyatt]

Fixed.