Remove [TreatNullAs=NullString] from title attribute on Document

Remove [TreatNullAs=NullString] from title attribute on Document

Current behavior:
Chrome: document.title=null => ""
Edge: document.title=null => "null"
Firefox Nightly: document.title=null => "null"

After this change, we will align with Edge and Firefox.

BUG=497307
Committed: https://crrev.com/e7eb96b6d15fe9e29b4723e72352bfd457d23697
Cr-Commit-Position: refs/heads/master@{#356838}

[tanay.c, 2015-10-27 09:20:32]

tanay.c@samsung.com changed reviewers:
+ philipj@opera.com

[tanay.c, 2015-10-27 09:20:32]

PTAL

[philipj_slow, 2015-10-27 09:24:02]

The document.cookie change was submitted once before, see my comment on
https://codereview.chromium.org/1157223007/

Similarly for the other two attributes, you need to analyze the code to see if
the change will be in any way observable to web contents. If it is, you need to
write it is, and if it isn't explain why in the description.

[tanay.c, 2015-10-27 11:37:06]

On 2015/10/27 09:24:02, philipj wrote:
> The document.cookie change was submitted once before, see my comment on
> https://codereview.chromium.org/1157223007/
> 
> Similarly for the other two attributes, you need to analyze the code to see if
> the change will be in any way observable to web contents. If it is, you need
to
> write it is, and if it isn't explain why in the description.

There are existing tests for checking null attributes in
/Blink/src/third_party/WebKit/LayoutTests/fast/dom/document-attribute-js-null.html

---
/home/tanay.c/Blink/src/webkit/Release/layout-test-results/fast/dom/document-attribute-js-null-expected.txt
+++
/home/tanay.c/Blink/src/webkit/Release/layout-test-results/fast/dom/document-attribute-js-null-actual.txt
@@ -8,14 +8,14 @@
 TEST SUCCEEDED: The value was the string 'UTF-8'. [tested
Document.inputEncoding]
 TEST SUCCEEDED: The value was null. [tested Document.selectedStylesheetSet]
 
-TEST SUCCEEDED: The value was the empty string. [tested HTMLDocument.title]
-TEST SUCCEEDED: The value was the empty string. [tested HTMLDocument.cookie]
+TEST FAILED: The value should have been null but was the string 'null'. [tested
HTMLDocument.title]
+TEST FAILED: The value should have been null but was the empty string. [tested
HTMLDocument.cookie]
 TEST SUCCEEDED: The value was the empty string. [tested HTMLDocument.bgColor]
 TEST SUCCEEDED: The value was the empty string. [tested HTMLDocument.fgColor]
 TEST SUCCEEDED: The value was the empty string. [tested
HTMLDocument.alinkColor]
 TEST SUCCEEDED: The value was the empty string. [tested HTMLDocument.linkColor]
 TEST SUCCEEDED: The value was the empty string. [tested
HTMLDocument.vlinkColor]
-TEST SUCCEEDED: The value was the empty string. [tested HTMLDocument.dir]
+TEST FAILED: The value should have been null but was the empty string. [tested
HTMLDocument.dir]
 TEST SUCCEEDED: The value was the string 'off'. [tested
HTMLDocument.designMode]

[philipj_slow, 2015-10-27 13:12:28]

OK, sounds like for title and dir all you'd need to do is to update those tests.

The most important thing to do first is to test the behavior of Firefox and
Edge. If they both treat null as "null" then the change should be very low risk.

[tanay.c, 2015-10-28 10:48:22]

Description was changed from

==========
Remove [TreatNullAs=NullString] from attributes on Document

BUG=497307
==========

to

==========
Remove [TreatNullAs=NullString] from title attribute on Document

Current behavior:
Firefox Nightly
document.title=null => "null"

Chrome:
document.title=null => ""

Behavior after change:
Firefox Nightly
document.title=null => "null"

Chrome:
document.title=null => "null"



BUG=497307
==========

[tanay.c, 2015-10-28 10:48:32]

On 2015/10/27 13:12:28, philipj wrote:
> OK, sounds like for title and dir all you'd need to do is to update those
tests.
> 
> The most important thing to do first is to test the behavior of Firefox and
> Edge. If they both treat null as "null" then the change should be very low
risk.

PTAL.

[philipj_slow, 2015-10-28 12:02:23]

Description was changed from

==========
Remove [TreatNullAs=NullString] from title attribute on Document

Current behavior:
Firefox Nightly
document.title=null => "null"

Chrome:
document.title=null => ""

Behavior after change:
Firefox Nightly
document.title=null => "null"

Chrome:
document.title=null => "null"



BUG=497307
==========

to

==========
Remove [TreatNullAs=NullString] from title attribute on Document

Current behavior:
Chrome: document.title=null => ""
Edge: document.title=null => "null"
Firefox Nightly: document.title=null => "null"

After this change, we will align with Edge and Firefox.

BUG=497307
==========

[philipj_slow, 2015-10-28 12:02:42]

On 2015/10/28 10:48:32, tanay.c wrote:
> On 2015/10/27 13:12:28, philipj wrote:
> > OK, sounds like for title and dir all you'd need to do is to update those
> tests.
> > 
> > The most important thing to do first is to test the behavior of Firefox and
> > Edge. If they both treat null as "null" then the change should be very low
> risk.
> 
> PTAL.

I've updated the description to also list Edge.

[philipj_slow, 2015-10-28 12:02:46]

The CQ bit was checked by philipj@opera.com

[philipj_slow, 2015-10-28 12:02:46]

lgtm

[commit-bot: I haz the power, 2015-10-28 12:03:06]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1415503007/20001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1415503007/20001

[philipj_slow, 2015-10-28 12:39:13]

The CQ bit was unchecked by philipj@opera.com

[philipj_slow, 2015-10-29 11:57:10]

The CQ bit was checked by philipj@opera.com

[commit-bot: I haz the power, 2015-10-29 11:57:29]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1415503007/20001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1415503007/20001

[commit-bot: I haz the power, 2015-10-29 14:37:41]

Committed patchset #2 (id:20001)

[commit-bot: I haz the power, 2015-10-29 14:38:39]

Patchset 2 (id:??) landed as
https://crrev.com/e7eb96b6d15fe9e29b4723e72352bfd457d23697
Cr-Commit-Position: refs/heads/master@{#356838}