It should be possible to set a document's title by setting title.text and it should also be possible to retreive it by getting title.text. This doesn't work right now.
Created attachment 2421 [details] Support text property on titles Here's a patch with test cases. I modified DumpRenderTree so it would (optionally) print out title changes.
Created attachment 2422 [details] Make DumpRenderTree optionally print out title changes
I believe Anders when he files a bug, will ask someone on the apple webkit team to give him canconfirm and editbugs privileges.
Created attachment 2435 [details] Reset value of dumpTitleChanges after each run We need to set dumpTitleChanges to NO after each run to make the other test cases pass.
I have an old title patch that addresses other problems with titles. I'm attaching it, just so Anders and other can see it.
Created attachment 2442 [details] Possibly-relevant title patch I had sitting around. This patch attempts to deal with the issue of competing attempts to set the title by setting the text of the title element, setting the title on the document element, and manipulating the children of the title element directly with DOM calls. But I don't think I have enough test cases to be sure it does the right thing.
Created attachment 2443 [details] Possibly-relevant title patch I had sitting around (uploading again). Trouble uploading. Trying again.
Comment on attachment 2435 [details] Reset value of dumpTitleChanges after each run This looks fine. I think it would be even more elegant to figure out a way to add this without a new method. For example, any "unexpected" title changes could be logged. Perhaps the code could check to see if the <title> element, if any, was parsed, and simply dump all title changes after that point. But I also think it's fine to land this as-is. r=me
Comment on attachment 2421 [details] Support text property on titles We can probably do even better later (see my patch), but this patch seems like a clear improvement. One area for future improvement is to share code with other places that need to concatenate text from all child elements. Maybe that's only the <script> tag? r=me
I do have a newer patch that incorporates your changes as well as adds a test case. I also noticed that WebKit doesn't handle setting the title to '' so I have a patch for that as well. I'll upload these patches tomorrow.
Sounds great!
Created attachment 2481 [details] Integrate Darin's patch and add tests
Here are a few things I've been thinking about: 1) Can we share code in the text/setText functions for HTMLOptionElementImpl, HTMLTitleElementImpl, and HTMLScriptElementImpl? 2) Should setText optimize for the case where the text isn't change and not send out childrenChanged and the DOM tree modification events? Does that break anything for <title>? For <script>? 3) The HTMLOptionElementImpl looks at the entire subtree, and doesn't assume that text nodes will all be at the top level. Do we want that for <title>? For <script>? If so, then childrenChanged() isn't a good enough notification since it doesn't bubble up to parents. 4) Do we want to add a new replaceChildren so we can remove the children and add the one new text node without sending out double childrenChanged and double DOM tree modification events? 5) Should we bother fixing the "removed a <title> element" case to look for a remaining <title> element? I think he has answers to all of these questions, and there's a chance we won't have to change the patch at all. It would also be good to ensure the test case covers enough of the corner cases, since the patch has a lot of different code paths. Maybe we're already there. I think I'm going to wait on reviewing this patch until we get a tiny bit more data.
(In reply to comment #13) > Here are a few things I've been thinking about: > > 1) Can we share code in the text/setText functions for HTMLOptionElementImpl, > HTMLTitleElementImpl, and HTMLScriptElementImpl? We could definitely share code for text in HTMLTitleElementImpl and HTMLScriptElementImpl. HTMLOptionElementImpl does work differently (As is pointed out in 3) but having something like DOMString NodeImpl::getChildrenTextContents(bool traverse) could work, I'm not sure. Looks like setText could be shared between all three implementations. > 2) Should setText optimize for the case where the text isn't change and not send out > childrenChanged and the DOM tree modification events? Does that break anything for <title>? For > <script>? I don't think that should break anything for <title> or <script>. The way ::text() and ::setText() works is highly imlementation-dependent, see my comment on 3) below. > 3) The HTMLOptionElementImpl looks at the entire subtree, and doesn't assume that text nodes will > all be at the top level. Do we want that for <title>? For <script>? If so, then childrenChanged() isn't a > good enough notification since it doesn't bubble up to parents. Like I stated before, the way ::text and ::setText work is very implementation-specific. Mozilla, for example does only look at the first child node's text contents, and doesn't allow you to set title.text unless its first child node is a text node. So something like <title><span>foo</span></title> created by the DOM could not be changed by setting title.text. WinIE does not allow you to change the title at all by appending children to the title element, only by setting title.text or document.title. So basically, I think what we have now should be good enough. > 4) Do we want to add a new replaceChildren so we can remove the children and add the one new text > node without sending out double childrenChanged and double DOM tree modification events? For the general case the <title> element should only have one child (I think this is always true when dealing with HTML, and it'll be true for XML documents as well unless there's a mix between text and cdata) in which case we'll just update the child node's text. I don't think it's worth adding a new method just for this. > 5) Should we bother fixing the "removed a <title> element" case to look for a remaining <title> > element? Neither Mozilla nor WinIE does this, and I can't think of a good use case for this. > > I think he has answers to all of these questions, and there's a chance we won't have to change the patch > at all. > > It would also be good to ensure the test case covers enough of the corner cases, since the patch has a > lot of different code paths. Maybe we're already there. I think I'm going to wait on reviewing this patch > until we get a tiny bit more data. I think what we have test-case wise is pretty good, although I did add spanElement = document.createElement('span'); spanElement.appendChild(document.createTextNode('4. appending a span element with a text child (should not trigger a title change)')); title.appendChild(spanElement); to the second test case.
Comment on attachment 2481 [details] Integrate Darin's patch and add tests OK, I'm convinced, r=me.
Radar for this is: <rdar://problem/3831364> getting or setting text for a <title> element doesn't work This patch also fixes this bug: <rdar://problem/4091225> REGRESSION (1.2.4-1.3): many titles with non-ASCII characters do not display properly (XHTML strict only) although this fix doesn't deal with the underlying problem that libxml2 breaks the title up into more than one text node, which is the other cause of that latter problem.
Created attachment 2484 [details] Test case for the non-ASCII XML title bug that this patch also fixes