If a title is changed in the following way <script language="JavaScript" type="text/javascript"> theTitle=document.getElementsByTagName("title").item(0).firstChild; document.write(theTitle.nodeValue, "<br />"); theTitle.replaceData(0, theTitle.nodeValue.length, "This is a new title."); document.write(theTitle.nodeValue); </script> then the title is changed in the document but not in the title bar of the document window nor in the history. Additional proposal: Don't add an additional history entry if the title is change, only change the title.

to DOM core....

Future.

changing priority to P5

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

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

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

Ok. But there is another backfire -- TITLE tag does not support id properety, so only way to gain acsess to it, use getElementsByTagName('title')[0]. getElementById does not work. Also w3c HTML 4.01 attributes dir and lang does not supported.

Well. As you see, text is only properety, wich supported (and changed getElementsByTagName('title')[0].text lead to change of document.tittle). All others no supported.

Hmm. Source of this behaviour lies in function: HTMLContentSink::AddHeadContent(const nsIParserNode& aNode) (/content/html/document/src/nsHTMLContentSink.cpp) 3533 nsHTMLTag type = nsHTMLTag(aNode.GetNodeType()); 3534 if (eHTMLTag_title == type) { 3535 nsCOMPtr<nsIDTD> dtd; 3536 mParser->GetDTD(getter_AddRefs(dtd)); 3537 if (dtd) { 3538 nsAutoString title; 3539 PRInt32 lineNo = 0; 3540 dtd->CollectSkippedContent(eHTMLTag_title, title, lineNo); 3541 rv = SetDocumentTitle(title); 3542 } 3543 } 3544 else { 3545 rv = AddLeaf(aNode); 3546 } Well, for <title> tag this code do following: it ignored ALL attributes, remember only title content. Even more, SetDocumentTitle(title); create a new HTMLTitleElement only for first <title> tag, trashed all other <title> tags. I don't think that this is aceptable.

There is only one <title> allowed per the HTML spec. Read the DTD. Or read section 7.4.3, which says "...the TITLE element, which provides information about an entire document and may only appear once..." Also, there are no ways to get the dir and lang attributes of the TITLE element other than using getAttribute. See http://www.w3.org/TR/2003/REC-DOM-Level-2-HTML-20030109/html.html#ID-79243169 which does not define anything for HTMLTitleElement other than text. Please don't make assumptions about what you think is acceptable or not without consulting the specs first.

Comment on attachment 112451 [details] HTMLTitleElenent propereties This testcase as written is invalid.

Sorry, Christopher, but you should consult manual more precisely. dir and lang is attributes of HTMLElement, and they DECLARED by line: interface HTMLTitleElement : HTMLElement and http://www.w3.org/TR/1999/REC-html401-19991224/struct/global.html#edef-TITLE: 7.4.2 The TITLE element <!-- The TITLE element is not considered part of the flow of text. It should be displayed, for example as the page header or window title. Exactly one title is required per document. --> <!ELEMENT TITLE - - (#PCDATA) -(%head.misc;) -- document title --> <!ATTLIST TITLE %i18n> Start tag: required, End tag: required Attributes defined elsewhere * lang (language information), dir (text direction) So dir and lang AND id! should be inherited from HTMLElement. Even more, in current version this attributes SUPPORTED but INCORRECTLY. Of you think, that id for tittle is not allowed? P.S. Where is stated that there should be exactly one title? There is only: Every HTML document must have a TITLE element in the HEAD section. Nothing more.

Aha, I found an answer to last question. So second title is realy incorect, and this part of guessing goeas away.

Comment on attachment 112451 [details] HTMLTitleElenent propereties Er, right, I forgot about HTMLElement. But still, I point you to the sentence I copied. Which as I said is in 7.4.3 (not 7.4.2 as you quoted. Look down just a few paragraphs).

Well, I realy think about such code: <title>This is element title</title> <script type="text/javascript"> var tt=document.createElement('title') document.getElementsByTagName("HEAD")[0].appendChild(tt) tt.text="I am the new title" </script> After it, there is 2 title elements in HTML file (I check it), and change text properety of each of it set document.title.

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

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

Mass-reassigning bugs.

Just click on every button...

id, dir, lang properties in attachment 112451 [details] work, if the file is served as application/xhtml+xml. Is Bug 280044 a dup of this?

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

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

The multiple-titles issue mentioned here seems to be generally agreed to be invalid, but the dynamic-changes issue is still valid, so I changed the summary to match the valid issue.

(In reply to comment #23) > but the dynamic-changes issue is still valid, so I changed the summary > document.title does not dynamically update when the contents > of the document's <title> element change Then, when title attribute changed, <title> element should update, or not?

(In reply to comment #24) > Then, when title attribute changed, <title> element should update, or not? Probably not, when you read an element you should only get its current DOM state, and document.title is a property of the document, not an element (sorry if that sounds confusing). Are you going to request review on this patch?

(In reply to comment #24) > Created an attachment (id=312294) [details] > <title> element -> document.title property > > (In reply to comment #23) > > but the dynamic-changes issue is still valid, so I changed the summary > > > document.title does not dynamically update when the contents > > of the document's <title> element change > > Then, when title attribute changed, <title> element should update, or not? > Actually, ignore my above comment, it should update. Opera and Webkit both do so, so I think we should too.

I think we need a little more work here. Setting document.title should change the contents of the <title> element, if there is one. I'll have a go.

The HTML5 spec says we should actually make document.title defer to the contents of the <title> element: http://www.whatwg.org/specs/web-apps/current-work/#document.title We should probably implement it that way and get rid of nsDocument::mDocumentTitle. Torisugari, do you want to carry on with this or should I take over? Your patch is a good start, but instead of calling nsHTMLDocument::SetTitle it should call nsDocument::SetTitle (probably renamed to SetTitleInternal and exposed through nsIDocument). We should get rid of mDocumentTitle. nsDocument::GetTitle and nsDocument::SetTitle should be reimplemented to use the HTML5 algorithm.

(In reply to comment #28) > Torisugari, do you want to carry on with this or should I take over? I'd like to try it out this weekend, unless you've already started writing patch.

Please go ahead!

Adding keyword 'html5' per comment 28

Gettin this on the 1.9.1 nom list.

O. Atsushi (Torisugari) did you get much further?

Another testcase for > 3. If the title element is null, then a new title element must be > created and appended to the head element.

I have a question. According to whatwg HTML5, *the title element* is defined in this way. > The title element of a document is the first title element in > the document (in tree order), if there is one, or null otherwise. That is, > <html> > <title>A</title> > <head> > <title>B</title> > </head> > <body> > <title>C</title> > <title>D</title> > <title>E</title> > </body> > </html> *The title element* is |<title>A</tiltle>|. So the title bar displays "A". This is OK. Then, a user can remove *the title element*. > var element = document.getElementsByTagName("title")[0]; > element.parentNode.removeChild(element); What should happen? In theory (probably), the moment removeChild() is called, *the title element* is |<title>B</title>|. i.e. removeChild() should change the title bar from "A" to "B"? Another question is, > document.getElementsByTagName("title")[2].text = "F"; can change the title bar? Or it should not? |document.getElementsByTagName("title")[2]| is not *the title element* while it's *a title element* nonetheless. Such behavior is not described in the whatwg spec. We should respect or ignore the third title element? Anyway, when given document has 2 or more <title> elements, it's a little confusing. Though I'm afraid less than 0.00000001% of users may have experienced removing <title> element from a HTML document.

(In reply to comment #35) > Then, a user can remove *the title element*. > > > var element = document.getElementsByTagName("title")[0]; > > element.parentNode.removeChild(element); > > What should happen? In theory (probably), the moment removeChild() is > called, *the title element* is |<title>B</title>|. > i.e. removeChild() should change the title bar from "A" to "B"? That's right. We want to avoid having a mutation listener on the entire document. So I think we'll need a hook in nsHTMLTitleElement::BindToTree/UnbindFromTree to detect when title elements are added or removed and update the window title. > Another question is, > > > document.getElementsByTagName("title")[2].text = "F"; > > can change the title bar? Or it should not? > > |document.getElementsByTagName("title")[2]| is not *the title element* > while it's *a title element* nonetheless. Such behavior is not described in > the whatwg spec. We should respect or ignore the third title element? Ignore it. The window title should always be based on whatever the title attribute is returning, and the title attribute depends only on "the title element" and its contents.

I don't know how you're doing this but IMHO the right way to implement it is to not store the "title attribute" anywhere. Just make GetTitle compute it from scratch by analyzing the DOM. And when the DOM changes in a way that could affect which element "the title element" is, or could affect the contents of "the title element", then we need to trigger updating of the window title (which will call GetTitle). SetTitle just needs to make the DOM changes that the HTML5 spec describes, and then the window title will update automatically.

roc - I'll take a look at this if you don't mind.

I'll work on this shortly.

Here's a working patch. The basic idea is described above. Instead of storing the title in a dedicated variable, it's just something we compute from the DOM tree. SetTitle modifies the DOM. The content sinks don't need to explicitly track the title anymore, we just have to make sure that the <title> element is properly notified so it can fire DOMTitleChanged when it should. This actually simplifies code. I've made DOMTitleChanged fully async, there doesn't seem to be any reason why it needs to e synchronous. One thing I know I haven't handled is the title change when a XUL root element is removed or added to the DOM. I just don't care enough to make that work.

So one way that *may* be simpler to keep track of the current title is this: 1. Make nsIDocument keep a (weak) pointer: nsIContent* mFirstTitleElement 2. Inside the title elements ::BindToTree call check if the documents mFirstTitleElement is set. If it's set, call nsContentUtils::PositionIsBefore and if the existing title is before abort. 3. Set the documents mFirstTitleElement 4. In title elements ::UnbindFromTree, walk the DOM to search for a new title and update mFirstTitleElement accordingly. Not sure if that's better or worse than what you're doing.

That sounds more complicated than what I'm doing. It would speed up GetTitle, but I don't think we should be concerned about that (at least until someone shows us a real-world testcase where it matters). The only possible GetTitle performance concern I can see is where nsDocShell::RestoreFromHistory calls NotifyPossibleTitleChange; if the document is large and has no title, the resulting call to GetTitle will scan the entire DOM. So we could slow down page restore a fraction for those pages. I'm not convinced that's something to be concerned about.

If that was a concern, my preferred solution would be to add a single bit to nsDocument which records whether there was ever an SVG or HTML <title> element bound to the document. Then GetTitle could use that to avoid scanning the whole document if we know there can't be a title element.

Comment on attachment 333329 [details] [diff] [review] fix - In nsDocument::GetHtmlChildContent(nsIAtom* aTag): + // Look for body inside html. This needs to run forwards to find + // the first body element. Fix this comment, we're not necessarily looking for "body" here. - In nsDocument::GetTitleContent(PRUint32 aNodeType): +{ + nsRefPtr<nsContentList> list = + NS_GetContentList(this, nsGkAtoms::title, kNameSpaceID_Unknown); Jonas brought up the idea that over all it'd probably be beneficial to keep a weak pointer to the first title element (or an array/list of all of them if need be) and have the notifications for added/(re)moved title elements tell the document what title element was added/(re)moved title elements use nsContentUtils::ComparePosition() to know whether the notification is for the first title or not etc. If we did that we'd only ever need to walk the DOM when a title element is removed from the DOM tree (unless we keep pointers to all title elements), which basically never happens. If we don't do that then this could get really expensive in the case where we're dealing with a really large document that has no title element. Not the normal case either, but definitely a possible case. - In nsDocument::GetTitleFromElement(): +{ + nsIContent* title = GetTitleContent(aNodeType); + if (!title) + return NS_OK; + nsContentUtils::GetNodeTextContent(title, PR_FALSE, aTitle); + return NS_OK; Either return what GetNodeTextContent() returns here, or make this a void function (the latter is probably preferred). - In nsRunnableMethod::Revoke(): + NS_IF_RELEASE(mObj); + mObj = nsnull; NS_IF_RELEASE(mObj) sets mObj to nsnull for you. Over all this looks really good. I'd like to look over an updated patch with the optimization to remember the current title element in the document before stamping this though.

Yeah, i'm not really concerned about performance about anything here. We should do whatever results in the least amount of code.

I'm quite sure that tracking the first title element will be more code. We'll still have to walk the document when a title element is removed (either SVG title or HTML title), so that code will still be around somewhere, but we'd have additional code to track the first title element. > If we don't do that then this could get really expensive in the case where > we're dealing with a really large document that has no title element. Note that in regular page load, such a document won't (well, "shouldn't", I haven't tested) fire a DOMTitleChanged event, so I wouldn't expect anyone to be calling GetTitle, unless chrome does it somewhere unexpected. I'll post a new patch with the other comments addressed.

Also note that if someone calls GetTitle more than once on the same document, then it seems the content list is likely to be cached so we should be pretty efficient there too.

Updated to comments, plus I added the 1-bit mMayHaveTitleElement optimization since it's very simple and should be completely effective for title-less documents.

Comment on attachment 333880 [details] [diff] [review] fix v2 Yup, makes sense. r+sr=jst

Ready to be landed? Without tests????

The tests are in the patch.

Ok, sorry I hadn't seen it.

So it looks like sometimes the code is looking for a <title> element by just looking through the children of <head>, sometimes by looking through the whole DOM. Is this intended? Sorry, just glanced over the patch.

Where are we only looking through the children of <head>? HTML5 says that "the title element" is the first <title> in the document, whether it's in the head or not. It also says that when you create a new title by setting .title, the new <title> element is appended to "the head element" (and *nothing happens* if there is no "the head element"). That should be the only dependence on <head> in this patch.

Yeah, you are right, i misread the part that inserts a new title when needed. Thought I had posted a comment fessing up to my mistake, but apparently not :)

Pushed 9934298811d7.

Backed out. I think this may have caused Windows orange due to leaks. I'm not sure what the problem is, but it could be that in NotifyPossibleTitleChange, NS_DispatchToCurrentThread could fail (because the current thread has shut down? I don't know), and in that case mPendingTitleChangeEvent stays around with a zero refcount, because it was never addrefed ... holding a strong ref to our nsDocument, so it and everything hanging off it is leaked. We should fix that by assigning the result of new nsRunnableMethod to an nsCOMPtr<nsIRunnable> in NotifyPossibleTitleChange, so if the dispatch fails then the refcount of the event will be decreased to zero as we leave NotifyPossibleTitleChange and the event will be destroyed. Of course mPendingTitleChangeEvent should only be set if dispatch succeeds. I'll reland with that fix later and see if that is the problem...

Acid 3 now says: Test 98 failed: doc.body is undefined Mozilla/5.0 (Windows; U; Windows NT 6.0; sk; rv:1.9.1a2pre) Gecko/20080816031323 Minefield/3.1a2pre

(In reply to comment #58) That's from bug 450160.

when I had used this patch (before it was pulled), a weird space did appear in the tab, http://img294.imageshack.us/img294/8725/titlespacevl5.jpg

> when I had used this patch (before it was pulled), a weird space did appear in > the tab, > > http://img294.imageshack.us/img294/8725/titlespacevl5.jpg > This seems to only be a problem with last.fm user music profiles. <title> mdew’s Music Profile – Users at Last.fm </title> shows in the source of the page so this would be an evangelism bug.

I don't think so. Mozilla should keep stripping the white space. This is what IE is doing and what Mozilla currently does. It seems like an unintended effect from the patch.

Should we strip the whitespace from document.title or in chrome when we display the title? What does IE do?

I tested this. FF3, IE7 and Opera 9.51 all compress internal whitespace and strip leading and trailing whitespace. Only Safari 3.1 does not. I posted to whatwg recommending that the spec change to make dominant UA behaviour. In the meantime, I'll reland with a call to aTitle.CompressWhitespace in nsDocument::GetTitle.

testcase for whitespace compression

I relanded, changeset c771615414fe.

For the record, in the initial landing the leak was triggered in mochitests.

(In reply to comment #67) > For the record, in the initial landing the leak was triggered in mochitests. good thing it was unlanded =P

This time looks better, tests passed on Windows.

Which version of Firefox will this fix be in? Thanks. (http://code.google.com/p/fbug/issues/detail?id=1315 Issue 1315: Changes made to the <title> tag are not seen.)

Given the target milestone of mozilla1.9.1a2, I'd guess Firefox 3.1 Alpha 2.