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

Issue 737133002: Make arguments to some Document and Element methods non-optional (Closed)

Created:
6 years, 1 month ago by philipj_slow
Modified:
6 years, 1 month ago
Reviewers:
pdr.
CC:
arv+blink, blink-reviews, blink-reviews-dom_chromium.org, Inactive, dglazkov+blink, eae+blinkwatch, rwlbuis, sof
Project:
blink
Visibility:
Public.

Description

Make arguments to some Document and Element methods non-optional https://dom.spec.whatwg.org/#document https://dom.spec.whatwg.org/#element The modified missing-arguments.html tests all pass in IE11 and Firefox Nightly 36.0a1, i.e. these arguments are already non-optional in those browsers, which limits the possible breakage to WebKit-specific content. Note that Document.createAttributeNS()'s qualifiedName argument should not be nullable per spec, but that is an orthogonal issue. BUG=325922 Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=185773

Patch Set 1 #

Patch Set 2 : add tests #

Patch Set 3 : test #

Patch Set 4 : restore nullability #

Patch Set 5 : update expectations #

Patch Set 6 : rebase+adapt #

Unified diffs Side-by-side diffs Delta from patch set Stats (+40 lines, -27 lines) Patch
M LayoutTests/fast/dom/Document/createAttributeNS-namespace-err-expected.txt View 1 2 3 4 5 1 chunk +0 lines, -2 lines 0 comments Download
M LayoutTests/fast/dom/Document/missing-arguments.html View 1 1 chunk +4 lines, -0 lines 0 comments Download
M LayoutTests/fast/dom/Document/missing-arguments-expected.txt View 1 2 3 4 1 chunk +4 lines, -0 lines 0 comments Download
M LayoutTests/fast/dom/Document/script-tests/createAttributeNS-namespace-err.js View 1 2 3 4 5 1 chunk +0 lines, -4 lines 0 comments Download
M LayoutTests/fast/dom/Element/missing-arguments.html View 1 2 chunks +7 lines, -1 line 0 comments Download
M LayoutTests/fast/dom/Element/missing-arguments-expected.txt View 1 2 3 4 2 chunks +7 lines, -0 lines 0 comments Download
M LayoutTests/fast/dom/wrapper-classes.html View 1 2 2 chunks +2 lines, -2 lines 0 comments Download
M LayoutTests/fast/dom/wrapper-classes-expected.txt View 1 2 2 chunks +8 lines, -8 lines 0 comments Download
M Source/core/dom/Document.idl View 1 2 3 1 chunk +3 lines, -4 lines 0 comments Download
M Source/core/dom/Element.idl View 2 chunks +5 lines, -6 lines 0 comments Download

Messages

Total messages: 12 (2 generated)
philipj_slow
add tests
6 years, 1 month ago (2014-11-19 23:25:33 UTC) #1
philipj_slow
test
6 years, 1 month ago (2014-11-20 14:10:44 UTC) #2
philipj_slow
restore nullability
6 years, 1 month ago (2014-11-20 14:31:55 UTC) #3
philipj_slow
PTAL. These are Web-facing changes, but on rather obscure APIs and breakage is unlikely, so ...
6 years, 1 month ago (2014-11-20 14:38:26 UTC) #5
philipj_slow
update expectations
6 years, 1 month ago (2014-11-20 19:37:53 UTC) #6
philipj_slow
There's a chance some test changed in https://codereview.chromium.org/740223003/ will break, so I'll wait before that's ...
6 years, 1 month ago (2014-11-20 19:39:36 UTC) #7
philipj_slow
rebase+adapt
6 years, 1 month ago (2014-11-20 23:31:01 UTC) #8
pdr.
lgtm
6 years, 1 month ago (2014-11-21 07:13:58 UTC) #10
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/737133002/100001
6 years, 1 month ago (2014-11-21 07:14:36 UTC) #11
commit-bot: I haz the power
6 years, 1 month ago (2014-11-21 08:45:19 UTC) #12
Message was sent while issue was closed.
Committed patchset #6 (id:100001) as
https://src.chromium.org/viewvc/blink?view=rev&revision=185773

Powered by Google App Engine
This is Rietveld 408576698