Update the document title using jQuery while using the overlay window

Good idea, and needs to be fixed!

+    
+    // Store the original document title
+    self.$originalTitle = $(document).attr('title');
 
     // Add title to close button features for accessibility.
     self.$dialogTitlebar.find('.ui-dialog-titlebar-close').attr('title', Drupal.t('Close'));
@@ -455,6 +458,9 @@ Drupal.overlay.bindChild = function (ifr
   self.$dialogTitlebar.find('.ui-dialog-title').focus();
   // Add a title attribute to the iframe for accessibility.
   self.$iframe.attr('title', Drupal.t('@title dialog', { '@title': iframeTitle }));
+  
+  // Update the page title
+  document.title = self.$iframe.attr('title') + ' | ' + self.$originalTitle;
 
   // Remove any existing shortcut button markup in the title section.
   self.$dialogTitlebar.find('.add-or-remove-shortcuts').remove();

Lots of whitespaces on your newlines, those need to be removed. Also, in the code, the iframe title attribute is set, which you are retrieving in your patch. That's a DOM manipulation, and those are costly (even though it is performed on a cached object). It would be better to save the Drupal.t('@title dialog', { '@title': iframeTitle }) in a separate variable, and use it for both setting the iframe attr title, and the document title.

document.title = self.$iframe.attr('title') + ' | ' + self.$originalTitle;

Both the iframe title and the original title? Isn't the iframe title the title of the loaded page? So isn't this somehow duplicating parts and creating an unnecessarily long title? Or is the iframe title something else...

Also, you aren't setting the title back when the overlay gets closed....

Successfully tested patch from #4 on:

IE8
Firefox 3.6.3
Safari 4.0.5
Opera 10.10
IE6
Chrome 4.1.249.1045 (42898)

It even works after translating it into German. ^^

Nice to have a test report! I think that there are still some rogue spaces on your newlines in the patch :). Other than that, it looks good. Again, haven't tried it because my dev pc is still broken.

If at least 1 other person tests it, and then sets it RTBC.....

Code looks good, and #5 confirms it works in popular browsers.

BarisW, I don't understand what bug you're talking about.

We should not strip HTML tags from the iframe title. Sometimes we want that.

Please don't CNW an RBTC patch if what you think "needs work" is something you consider to be a new bug that deserves its own issue.

Cool patch, but do we really want to put the word "dialog" in every page title like that? That seems like jargon to me. Instead of "Structure dialog", "Content dialog", etc, don't we rather want "Structure | My Site", "Content | My Site", etc (i.e., the same title they would see without the overlay)? I think it's somewhat important to keep the name of the site in there...

For the issue of HTML in the iframe title, see #725734: Overlay doesn't escape any page titles (residual cleanup) where that is being handled. That might help here as well, since it provides a mechanism to pass data about the title from the child window to the parent window's JS.

looks simple enough.

The only thing that bothers me now: html is displayed in the title. Would it be possible, to not display html? If there is no way to do this, I'd RTBC this.

Without whitespace and adds the missing periods at the end of comments.
I stripped the HTML using a regex stolen from Drupal.ajaxError in drupal.js.
I have no clue whether the regex is too permissive or insecure or whatever, but testing the page in #17 now displays the title correctly.

I just noticed that regarding the page title, there is already an issue for that, or at least a related issue. I'll let someone who has spent more than 15 minutes in overlay's code decide whether to fix it here or there or whatever.
#725734: Overlay doesn't escape any page titles (residual cleanup)

A document with a <title> containing html elements is invalid. So it shouldn't be fixed here.

All right. This is #16 without whitespace and with periods.

I think you should change $originalTitle to originalTitle (we use $variable for jquery objects).

$(document).attr('title') should be document.title; The title element is no attribute of document (which isnt even an element). Besides, document.title is totally crossbrowser.

I know $(document).attr('title') is currently also used for var iframeTitle, but I changed that in #668640: Overlay shouldn't be based on jQuery UI Dialog.

Incorporates #22.
Untested.

Things need to be done a little different as #668640: Overlay shouldn't be based on jQuery UI Dialog landed.

This still has the problem that it puts HTML in the page title, and (as per #15) doesn't use the full Drupal page title (missing the "... | My Site Name" part).

However, when combined with the latest patch at #725734: Overlay doesn't escape any page titles (residual cleanup) it seems to work perfectly, so we might be good to go here as long as both of them get in :)

RTBC?

#725734: Overlay doesn't escape any page titles (residual cleanup) is in, so this one is rtbc by david (#25)

Yup, agreed with RTBC. The code looks good and it now works perfectly!

Still applies.

Committed to CVS HEAD. Thanks.