(Not really docshell but I don't know where appshell bugs get filed) bug 22183 puts the domain in the titlebar when a browser window has no location bar. On Mac OSX nsContentTreeOwner::mTitleSeparator is not set (windows don't show "- Mozilla" at the end) so the new domain runs directly into the start of the title. e.g. "https://bugzilla.mozilla.orgEnter Bug: Core"

presuming blocking status to make sure this is on the radar for Monday's drivers meeting.

Don't we need an equivalent fix for the tabbrowser code?

You'd think, but apparently tabbrowser is "broken" on MacOSX to behave like all the other platforms. See step 5 of https://bugzilla.mozilla.org/show_bug.cgi?id=22183#c247

Comment on attachment 174914 [details] [diff] [review] Add separator to mac title ok, sr=bzbarsky (and file a bug on tabbrowser on mac?)

I owe you a real explanation... On Mac nsContentTreeOwner::XULWindow copies the window modifier text ("Mozilla Firefox") into the default title (if empty) and actually removes the modifier attribute from the docshellElement. XULWindow() doesn't remove the separator from the docshell, but doesn't set nsContentTreeOwner's mTitleSeparator member in the Mac's case since it's never used. In my nsContentTreeOwner changes I was relying on mTitleSeparator, which was empty on Mac and caused the run-on. The change here gets the separator attribute from the docshell. A smaller change would have been to go ahead and set mTitleSeparator even on the mac, then my code in ::SetTitle() wouldn't need any changes. Over in the tabbrowser the title is constructed, but if modifier is null (and on Mac the content tree owner made sure it was) then the separator+modifier isn't added to the end. But the separator attribute my tabbrowser changes were using *did* exist so the bug did not occur when the tabbrowser was setting the title. This is a smaller change, do you prefer it? This always sets mTitleSeparator so it's available for my new code. The added check for the empty modifier in SeTitle shouldn't be necessary if the Mac is setting the default title, but seems safer anyway just in case some redistributor doesn't like branding their browser windows and has no modifier.

So does this last patch change the default behavior on mac in any way?

The change unconditionally sets nsContentTreeOwner::mTitleSeparator rather than leave it blank on the Mac. The only place that used this member besides my new code is seen in the patch--the reason I gave so much context. On the Mac we never get there because the existing ifdef mac code sets the default title, and for good measure I added a check for an empty modifier which the Mac code also zaps. I like the second patch simply because there's less conditional code and it eliminates the need for a couple temp autoStrings, but either one works. The second also eliminates the extra GetAttribute every time a title is set on the Mac, but I don't know if that was worth worrying about.

Comment on attachment 174935 [details] [diff] [review] alternate smaller patch sr=bzbarsky

Comment on attachment 174935 [details] [diff] [review] alternate smaller patch Yum, this version is much better, though I wonder why FireFox didn't just #ifdef the attributes in the XUL in the first place...

I filed bug 283000 on the lousy content title setting code.

Comment on attachment 174914 [details] [diff] [review] Add separator to mac title We're going with the other one

Comment on attachment 174935 [details] [diff] [review] alternate smaller patch a=caillon for the branches

Fix checked into 1.7 and aviary1.0.1 branches.

verified fixed using the tests in bug 22183 comment 247. there are no longer run-ons btwn the site name and the title. looks good!

note, I tested this on Mac OS X 10.3.8 with 2005022304-1.0.1 firefox bits.

Comment on attachment 174935 [details] [diff] [review] alternate smaller patch fixes bug 312426

fix checked into 1.8 branch