RESOLVED FIXED Bug 57537
Setting document.title doesn't affect contents of title tag of XHTML documents 
https://bugs.webkit.org/show_bug.cgi?id=57537
Summary Setting document.title doesn't affect contents of title tag of XHTML documents
Evan Martin
Reported 2011-03-31 03:41:06 PDT
This snippet has different behavior in HTML vs XHTML documents: <head><title id='t'>test</title></head> <body> <script>document.title = 'foo'; console.log(document.getElementById('t').innerText);</script> In HTML, we update the <title> tag. In XHTML, we don't.
Attachments
Patch (1.24 KB, patch)
2011-09-09 12:53 PDT, Jacky Jiang 		no flags
DetailsFormatted DiffDiff
Patch (1.38 KB, patch)
2011-09-09 13:18 PDT, Rob Buis 		no flags
DetailsFormatted DiffDiff
Patch (1.38 KB, patch)
2011-09-09 13:48 PDT, Rob Buis 		ap: review-
DetailsFormatted DiffDiff
Patch (3.95 KB, patch)
2011-09-09 18:55 PDT, Jacky Jiang 		no flags
DetailsFormatted DiffDiff
Patch (3.92 KB, patch)
2011-09-12 13:50 PDT, Jacky Jiang 		no flags
DetailsFormatted DiffDiff
Show Obsolete (3) View All Add attachment proposed patch, testcase, etc.
Evan Martin
Comment 1 2011-03-31 03:43:01 PDT
The problem is in Document::setTitle(), which tests isHTMLDocument, which isn't true for XHTML documents.
Jacky Jiang
Comment 2 2011-09-08 17:29:50 PDT
This in XHTML document works fine on Opera. Is there a specific reason that we are not taking XHTML documents into account in Document::setTitle()? It looks that should be the same with HTML documents.
Jacky Jiang
Comment 3 2011-09-09 12:53:38 PDT
Created attachment 106907 [details] Patch
Rob Buis
Comment 4 2011-09-09 13:18:29 PDT
Created attachment 106910 [details] Patch
Rob Buis
Comment 5 2011-09-09 13:35:20 PDT
(In reply to comment #4) > Created an attachment (id=106910) [details] > Patch For anyone curious, Jacky is not a committer so I uploaded the patch for him, trying to get EWS results.... Ignore for now. Cheers, Rob.
Rob Buis
Comment 6 2011-09-09 13:48:24 PDT
Created attachment 106916 [details] Patch
Alexey Proskuryakov
Comment 7 2011-09-09 15:28:35 PDT
Comment on attachment 106916 [details] Patch Please add a a test.
Jacky Jiang
Comment 8 2011-09-09 15:29:50 PDT
(In reply to comment #7) > (From update of attachment 106916 [details]) > Please add a a test. I'll add it.
Jacky Jiang
Comment 9 2011-09-09 18:55:23 PDT
Created attachment 106953 [details] Patch
Jacky Jiang
Comment 10 2011-09-09 18:57:15 PDT
Patch with a test case.
Darin Adler
Comment 11 2011-09-09 20:16:19 PDT
This patch makes me curious about how many of the 66 call sites of isHTMLDocument() are mishandling XHTML documents.
Alexey Proskuryakov
Comment 12 2011-09-09 20:57:20 PDT
Comment on attachment 106953 [details] Patch What is the reason why you didn't use regular WebKit script test machinery? That would make the test much shorter and more consistent with others. See e.g. <http://trac.webkit.org/browser/trunk/LayoutTests/fast/dom/allowed-children.html>.
Jacky Jiang
Comment 13 2011-09-12 08:05:05 PDT
(In reply to comment #12) > (From update of attachment 106953 [details]) > What is the reason why you didn't use regular WebKit script test machinery? That would make the test much shorter and more consistent with others. See e.g. <http://trac.webkit.org/browser/trunk/LayoutTests/fast/dom/allowed-children.html>. Ah, I see, I followed some other tests under dom directory. Thanks for your advice, I will take the advice and update the test case then.
Jacky Jiang
Comment 14 2011-09-12 13:50:36 PDT
Created attachment 107078 [details] Patch
Alexey Proskuryakov
Comment 15 2011-09-12 14:58:53 PDT
Comment on attachment 107078 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=107078&action=review Looks good, thank you! As Darin said, there can be a lot of interesting finds around other places that call isHTMLDocument(). > LayoutTests/fast/dom/title-content-set-innerText-get.xhtml:12 > +<p id="description"></p> > +<div id="console"></div> > +<script> > +description("This test ensures that we can update the contents of the title tag of the XHTML document when setting document.title"); This is how it's done it the test I gave a link to, but there is really no reason to set the description via a JavaScript function - one can just put it directly in a <p> element.
Jacky Jiang
Comment 16 2011-09-12 15:18:02 PDT
(In reply to comment #15) > (From update of attachment 107078 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=107078&action=review > > Looks good, thank you! > > As Darin said, there can be a lot of interesting finds around other places that call isHTMLDocument(). > > > LayoutTests/fast/dom/title-content-set-innerText-get.xhtml:12 > > +<p id="description"></p> > > +<div id="console"></div> > > +<script> > > +description("This test ensures that we can update the contents of the title tag of the XHTML document when setting document.title"); > > This is how it's done it the test I gave a link to, but there is really no reason to set the description via a JavaScript function - one can just put it directly in a <p> element. Thanks for your grant! If I find other places that call isHTMLDocument and mishandling XHTML documents, I'll file bugs and try to fix them.
WebKit Review Bot
Comment 17 2011-09-12 20:49:06 PDT
Comment on attachment 107078 [details] Patch Clearing flags on attachment: 107078 Committed r95009: <http://trac.webkit.org/changeset/95009>
WebKit Review Bot
Comment 18 2011-09-12 20:49:13 PDT
All reviewed patches have been landed. Closing bug.
Note You need to log in before you can comment on or make changes to this bug.