Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(32)

Issues Search
    My Issues | Starred     Open | Closed | All

Issue 98373012: Don't set document.title when there is no <head> (Closed)

Created:
7 years ago by davve
Modified:
6 years, 11 months ago
CC:
blink-reviews, dglazkov+blink, eae+blinkwatch, adamk+blink_chromium.org, wjmaclean
Base URL:
https://chromium.googlesource.com/chromium/blink.git@master
Visibility:
Public.

Description

Don't set document.title when there is no <head> Align setting document.title with http://www.whatwg.org/specs/web-apps/current-work/multipage/dom.html#document.title a bit closer by refusing to update the document title when there is no head element. BUG=330165 Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=164582

Patch Set 1 #

Total comments: 2
Unified diffs Side-by-side diffs Delta from patch set Stats (+24 lines, -5 lines) Patch
A LayoutTests/fast/dom/document-set-title-no-head.html View 1 chunk +17 lines, -0 lines 0 comments Download
A + LayoutTests/fast/dom/document-set-title-no-head-expected.txt View 1 chunk +2 lines, -3 lines 0 comments Download
M Source/core/dom/Document.cpp View 1 chunk +5 lines, -2 lines 2 comments Download

Messages

Total messages: 11 (0 generated)
davve
Found when investigating https://code.google.com/p/chromium/issues/detail?id=330140 One natural extension of this may be to remove the double ...
7 years ago (2013-12-20 12:59:55 UTC) #1
sof
On 2013/12/20 12:59:55, David Vest wrote: > Found when investigating > https://code.google.com/p/chromium/issues/detail?id=330140 > > One ...
7 years ago (2013-12-20 13:03:59 UTC) #2
davve
On 2013/12/20 13:03:59, sof wrote: > Have you checked compat of what the spec is ...
7 years ago (2013-12-20 13:17:16 UTC) #3
davve
https://codereview.chromium.org/98373012/diff/1/Source/core/dom/Document.cpp File Source/core/dom/Document.cpp (right): https://codereview.chromium.org/98373012/diff/1/Source/core/dom/Document.cpp#newcode1303 Source/core/dom/Document.cpp:1303: // http://www.whatwg.org/specs/web-apps/current-work/multipage/dom.html#document.title I'm a bit unsure about adding this ...
7 years ago (2013-12-20 14:39:12 UTC) #4
jochen (gone - plz use gerrit)
lgtm https://codereview.chromium.org/98373012/diff/1/Source/core/dom/Document.cpp File Source/core/dom/Document.cpp (right): https://codereview.chromium.org/98373012/diff/1/Source/core/dom/Document.cpp#newcode1303 Source/core/dom/Document.cpp:1303: // http://www.whatwg.org/specs/web-apps/current-work/multipage/dom.html#document.title I think it's fine to leave ...
6 years, 11 months ago (2014-01-03 11:04:45 UTC) #5
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/davve@opera.com/98373012/1
6 years, 11 months ago (2014-01-07 09:51:38 UTC) #6
commit-bot: I haz the power
Change committed as 164582
6 years, 11 months ago (2014-01-07 14:01:44 UTC) #7
adamk
This is causing failures in Chromium browser_tests. In particular: ClickToPlayPluginTest.NoCallbackAtLoad relies on setting document.title. See ...
6 years, 11 months ago (2014-01-07 22:19:51 UTC) #8
adamk
On 2014/01/07 22:19:51, adamk wrote: > This is causing failures in Chromium browser_tests. In particular: ...
6 years, 11 months ago (2014-01-07 22:23:10 UTC) #9
adamk
On 2014/01/07 22:23:10, adamk wrote: > On 2014/01/07 22:19:51, adamk wrote: > > This is ...
6 years, 11 months ago (2014-01-07 22:34:37 UTC) #10
davve
6 years, 11 months ago (2014-01-08 08:26:27 UTC) #11 
adamk@chromium.org writes:

> On 2014/01/07 22:23:10, adamk wrote:
>> On 2014/01/07 22:19:51, adamk wrote:
>> > This is causing failures in Chromium browser_tests. In particular:
>> >
>> >  ClickToPlayPluginTest.NoCallbackAtLoad
>> >
>> > relies on setting document.title. See failures:
>> >
>> >
>
>
http://build.chromium.org/p/chromium.webkit/builders/Mac10.6%252520Tests/buil...
>> >
>> > I'd like to roll out for now.
>
>> Hmm, Peter Beverloo landed a fix for the other test broken by this change:
>
>> http://src.chromium.org/viewvc/chrome?revision=243317&view=revision
>
>> so perhaps we can just fix the test. Investigating...
>
> As this is blocking the Blink roll and we're more than 100 revs behind, I'm
> going to roll out in https://codereview.chromium.org/109823011
>
> https://codereview.chromium.org/98373012/

Thanks for handling this. I'll investigate
ClickToPlayPluginTest.NoCallbackAtLoad further.

David

To unsubscribe from this group and stop receiving emails from it, send an email
to blink-reviews+unsubscribe@chromium.org.

Powered by Google App Engine
This is Rietveld 408576698