Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2791373002:
[InputEvent] Make StaticRange immutable and move tests to wpt (Closed)
Description[InputEvent] Make StaticRange immutable and move tests to wpt
According to the intent-to-ship in blink-dev there is still ongoing discussion about
StaticRange on whether we should make attributes mutable.
However this shouldn't block InputEvent and we should ship the immutable version
first to keep interop with WebKit.
This CL removed |Constructor()| |setStart()|, |setEnd()|, |toRange()| and made
attributes readonly.
See WebKit IDL:
https://github.com/WebKit/webkit/blob/1bfaaab3e7bf61dcad32c44a1399914987f9018f/Source/WebCore/dom/StaticRange.idl
StaticRange spec:
http://garykac.github.io/staticrange/index.html#interface-staticrange
Discussion:
https://groups.google.com/a/chromium.org/d/msg/blink-dev/W-Q1yWW-zas/UrqnyVAIDQAJ
BUG=707913
Review-Url: https://codereview.chromium.org/2791373002
Cr-Commit-Position: refs/heads/master@{#464665}
Committed: https://chromium.googlesource.com/chromium/src/+/e2b8278dd5aaf5a6ec415aa5e7dd828f17b644f6
Patch Set 1 #
Total comments: 10
Patch Set 2 : Make attributes immutable and address foolip's comments #
Total comments: 8
Patch Set 3 : Remove StaticRange constructor #
Total comments: 2
Patch Set 4 : foolip's comment: Remove TODO #Patch Set 5 : Fix global-interface-listing #Messages
Total messages: 62 (38 generated)
The CQ bit was checked by chongz@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
Description was changed from ========== Move StaticRange tests to wpt BUG= ========== to ========== [InputEvent] Move StaticRange layout tests to wpt BUG=707913 ==========
chongz@chromium.org changed reviewers: + foolip@chromium.org
foolip@ PTAL, thanks!
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
Do we have a consensus that StaticRange definition is merged into the DOM spec? wpt/dom/ is for the DOM spec.
chongz@chromium.org changed reviewers: + tkent@chromium.org
On 2017/04/03 21:37:36, tkent wrote: > Do we have a consensus that StaticRange definition is merged into the DOM spec? > wpt/dom/ is for the DOM spec. Ah sorry I didn't know that, I assumed StaticRange should live with Range... Do you have a recommended location? (e.g. Maybe wpt/staticrange/?) +garykac@ for more input.
On 2017/04/03 21:47:21, chongz wrote: > On 2017/04/03 21:37:36, tkent wrote: > > Do we have a consensus that StaticRange definition is merged into the DOM > spec? > > wpt/dom/ is for the DOM spec. > > Ah sorry I didn't know that, I assumed StaticRange should live with Range... Do > you have a recommended location? (e.g. Maybe wpt/staticrange/?) > > +garykac@ for more input. There isn't a "web-platform" directory, so I think that "uievents" would be the best location (since this is needed for Input events). Another option would be in "editing", but StaticRange is not intended to be editing-specific, so I think that uievents is better. Also, it is worth having the tests duplicated in fast/dom? I can see why we had that in the past, but I'm not sure about their value if we have coverage in WPT.
At this moment, StaticRange is defined only by https://garykac.github.io/staticrange/, right? We should create wpt/saticrange/. If StaticRange interface definition is merged into another specification, we should move wpt/staticrange/. We shouldn't add StaticRange tests to dom, uievents, editing, etc. even thought StaticRange is related to them because these specifications don't define StaticRange.
As tkent suggests, a new top-level staticrange directory would be appropriate for this. https://chromium.googlesource.com/chromium/src/+/master/docs/testing/web_plat... suggests two ways of doing it.
Can you also add a test using idlharness.js? https://codereview.chromium.org/2791373002/diff/1/third_party/WebKit/LayoutTe... File third_party/WebKit/LayoutTests/external/wpt/dom/ranges/StaticRange-constructor.html (right): https://codereview.chromium.org/2791373002/diff/1/third_party/WebKit/LayoutTe... third_party/WebKit/LayoutTests/external/wpt/dom/ranges/StaticRange-constructor.html:20: }, 'Default attributes'); Suggest 'Attribute default values', the attributes themselves are always the same. https://codereview.chromium.org/2791373002/diff/1/third_party/WebKit/LayoutTe... File third_party/WebKit/LayoutTests/external/wpt/dom/ranges/StaticRange-mutable.html (right): https://codereview.chromium.org/2791373002/diff/1/third_party/WebKit/LayoutTe... third_party/WebKit/LayoutTests/external/wpt/dom/ranges/StaticRange-mutable.html:21: }, 'Property mutable'); Suggest 'Mutable attributes' or 'Attributes are mutable' https://codereview.chromium.org/2791373002/diff/1/third_party/WebKit/LayoutTe... third_party/WebKit/LayoutTests/external/wpt/dom/ranges/StaticRange-mutable.html:33: }, 'setStart, setEnd'); Can you split this into two separate tests, and assert that setStart() has no effect on endContainer/endOffset and vice versa? Also, test what happens when startOffset is set to -1 or a number >= https://dom.spec.whatwg.org/#concept-node-length https://codereview.chromium.org/2791373002/diff/1/third_party/WebKit/LayoutTe... File third_party/WebKit/LayoutTests/external/wpt/dom/ranges/StaticRange-no-mutation.html (right): https://codereview.chromium.org/2791373002/diff/1/third_party/WebKit/LayoutTe... third_party/WebKit/LayoutTests/external/wpt/dom/ranges/StaticRange-no-mutation.html:15: const range = staticRange.toRange(); Add a test for when this throws an exception, or remove that bit of the IDL if it cannot. https://codereview.chromium.org/2791373002/diff/1/third_party/WebKit/LayoutTe... third_party/WebKit/LayoutTests/external/wpt/dom/ranges/StaticRange-no-mutation.html:29: }, 'StaticRange does not mutate on DOM changes'); If you make this the <title> you can omit it here, document.title is used as the default. (Or use the same title in both places.)
On 2017/04/04 04:24:04, foolip_UTC7 wrote: > As tkent suggests, a new top-level staticrange directory would be appropriate > for this. > https://chromium.googlesource.com/chromium/src/+/master/docs/testing/web_plat... > suggests two ways of doing it. The wpt-importer bot seems to be failing and won't import the staticrange/ directory. Should I wait until it's fixed or try manual importing? See https://build.chromium.org/p/chromium.infra.cron/builders/wpt-importer
On 2017/04/06 19:02:10, chongz wrote: > On 2017/04/04 04:24:04, foolip_UTC7 wrote: > > As tkent suggests, a new top-level staticrange directory would be appropriate > > for this. > > > https://chromium.googlesource.com/chromium/src/+/master/docs/testing/web_plat... > > suggests two ways of doing it. > > The wpt-importer bot seems to be failing and won't import the staticrange/ > directory. Should I wait until it's fixed or try manual importing? > > See https://build.chromium.org/p/chromium.infra.cron/builders/wpt-importer Have you added staticrange/ to W3CImportExpectations? Everything not listed there is skipped.
On 2017/04/06 at 19:02:10, chongz wrote: > The wpt-importer bot seems to be failing and won't import the staticrange/ directory. Should I wait until it's fixed or try manual importing? Yeah, it failed since csswg-test merger, and it was fixed today. I hope next import will have new directories. Manual import would be acceptable.
The CQ bit was checked by chongz@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: Try jobs failed on following builders: win_chromium_rel_ng on master.tryserver.chromium.win (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_rel_...)
The CQ bit was checked by chongz@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
Patchset #2 (id:20001) has been deleted
Description was changed from ========== [InputEvent] Move StaticRange layout tests to wpt BUG=707913 ========== to ========== [InputEvent] Make StaticRange immutable and move tests to wpt According to the intent-to-ship in blink-dev there is still ongoing discussion about StaticRange on whether we should make attributes mutable. However this shouldn't block InputEvent and we should ship the immutable version first to keep interop with WebKit. Discussion: https://groups.google.com/a/chromium.org/d/msg/blink-dev/W-Q1yWW-zas/UrqnyVAI... BUG=707913 ==========
Description was changed from ========== [InputEvent] Make StaticRange immutable and move tests to wpt According to the intent-to-ship in blink-dev there is still ongoing discussion about StaticRange on whether we should make attributes mutable. However this shouldn't block InputEvent and we should ship the immutable version first to keep interop with WebKit. Discussion: https://groups.google.com/a/chromium.org/d/msg/blink-dev/W-Q1yWW-zas/UrqnyVAI... BUG=707913 ========== to ========== [InputEvent] Make StaticRange immutable and move tests to wpt According to the intent-to-ship in blink-dev there is still ongoing discussion about StaticRange on whether we should make attributes mutable. However this shouldn't block InputEvent and we should ship the immutable version first to keep interop with WebKit. This CL removed |setStart()|, |setEnd()|, |toRange()| and made attributes readonly. See WebKit IDL: https://github.com/WebKit/webkit/blob/1bfaaab3e7bf61dcad32c44a1399914987f9018... Discussion: https://groups.google.com/a/chromium.org/d/msg/blink-dev/W-Q1yWW-zas/UrqnyVAI... BUG=707913 ==========
chongz@chromium.org changed reviewers: + qyearsley@chromium.org
foolip@ PTAL again, thanks! Also please see updated CL description. > Can you also add a test using idlharness.js? Done. I created the directory directly as we don't import directory with only OWNER file (According to the email with qyearsley@). https://codereview.chromium.org/2791373002/diff/1/third_party/WebKit/LayoutTe... File third_party/WebKit/LayoutTests/external/wpt/dom/ranges/StaticRange-constructor.html (right): https://codereview.chromium.org/2791373002/diff/1/third_party/WebKit/LayoutTe... third_party/WebKit/LayoutTests/external/wpt/dom/ranges/StaticRange-constructor.html:20: }, 'Default attributes'); On 2017/04/04 04:32:36, foolip_UTC7 wrote: > Suggest 'Attribute default values', the attributes themselves are always the > same. Done. https://codereview.chromium.org/2791373002/diff/1/third_party/WebKit/LayoutTe... File third_party/WebKit/LayoutTests/external/wpt/dom/ranges/StaticRange-mutable.html (right): https://codereview.chromium.org/2791373002/diff/1/third_party/WebKit/LayoutTe... third_party/WebKit/LayoutTests/external/wpt/dom/ranges/StaticRange-mutable.html:21: }, 'Property mutable'); On 2017/04/04 04:32:36, foolip_UTC7 wrote: > Suggest 'Mutable attributes' or 'Attributes are mutable' Changed to test "Attributes are immutable". https://codereview.chromium.org/2791373002/diff/1/third_party/WebKit/LayoutTe... third_party/WebKit/LayoutTests/external/wpt/dom/ranges/StaticRange-mutable.html:33: }, 'setStart, setEnd'); On 2017/04/04 04:32:36, foolip_UTC7 wrote: > Can you split this into two separate tests, and assert that setStart() has no > effect on endContainer/endOffset and vice versa? > > Also, test what happens when startOffset is set to -1 or a number >= > https://dom.spec.whatwg.org/#concept-node-length Removed |setStart/End()|. https://codereview.chromium.org/2791373002/diff/1/third_party/WebKit/LayoutTe... File third_party/WebKit/LayoutTests/external/wpt/dom/ranges/StaticRange-no-mutation.html (right): https://codereview.chromium.org/2791373002/diff/1/third_party/WebKit/LayoutTe... third_party/WebKit/LayoutTests/external/wpt/dom/ranges/StaticRange-no-mutation.html:15: const range = staticRange.toRange(); On 2017/04/04 04:32:36, foolip_UTC7 wrote: > Add a test for when this throws an exception, or remove that bit of the IDL if > it cannot. Removed |toRange()|. https://codereview.chromium.org/2791373002/diff/1/third_party/WebKit/LayoutTe... third_party/WebKit/LayoutTests/external/wpt/dom/ranges/StaticRange-no-mutation.html:29: }, 'StaticRange does not mutate on DOM changes'); On 2017/04/04 04:32:36, foolip_UTC7 wrote: > If you make this the <title> you can omit it here, document.title is used as the > default. (Or use the same title in both places.) Removed this test as we have no way to set the attributes. The correctness of StaticRange was tested in: https://cs.chromium.org/chromium/src/third_party/WebKit/Source/core/dom/Stati...
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
qyearsley@chromium.org changed reviewers: - qyearsley@chromium.org
LGTM, I think we can expect an export CL to be made to upstream this after this is committed. (Nit: Not important for this CL, but it still might be good to make a change to W3CImportExpectations to make it explicit that we want to sync the directory wpt/staticrange/.)
lgtm % constructor, but without the constructor none of the other tests are possible to write, so resolving https://github.com/garykac/staticrange/issues/5 first is a good idea. https://codereview.chromium.org/2791373002/diff/40001/third_party/WebKit/Layo... File third_party/WebKit/LayoutTests/external/wpt/staticrange/StaticRange-constructor.html (right): https://codereview.chromium.org/2791373002/diff/40001/third_party/WebKit/Layo... third_party/WebKit/LayoutTests/external/wpt/staticrange/StaticRange-constructor.html:7: assert_equals(typeof new StaticRange, 'object'); The constructor is gone from http://garykac.github.io/staticrange/index.html#interface-staticrange I've filed https://github.com/garykac/staticrange/issues/5 https://codereview.chromium.org/2791373002/diff/40001/third_party/WebKit/Layo... File third_party/WebKit/LayoutTests/external/wpt/staticrange/idlharness.html (right): https://codereview.chromium.org/2791373002/diff/40001/third_party/WebKit/Layo... third_party/WebKit/LayoutTests/external/wpt/staticrange/idlharness.html:14: [Constructor, Match the spec byte-for-byte if possible. (It doesn't have a constructor now.)
qyearsley@chromium.org changed reviewers: + qyearsley@chromium.org
https://codereview.chromium.org/2791373002/diff/40001/third_party/WebKit/Layo... File third_party/WebKit/LayoutTests/external/wpt/staticrange/staticrange-immutable.html (right): https://codereview.chromium.org/2791373002/diff/40001/third_party/WebKit/Layo... third_party/WebKit/LayoutTests/external/wpt/staticrange/staticrange-immutable.html:27: }); BTW, is it correct that we're not giving this subtest a name since there's only one subtest in this test file and it the test already has a title "StaticRange: Attributes are immutable"?
https://codereview.chromium.org/2791373002/diff/40001/third_party/WebKit/Layo... File third_party/WebKit/LayoutTests/external/wpt/staticrange/staticrange-immutable.html (right): https://codereview.chromium.org/2791373002/diff/40001/third_party/WebKit/Layo... third_party/WebKit/LayoutTests/external/wpt/staticrange/staticrange-immutable.html:27: }); On 2017/04/11 21:18:46, qyearsley wrote: > BTW, is it correct that we're not giving this subtest a name since there's only > one subtest in this test file and it the test already has a title "StaticRange: > Attributes are immutable"? I was following the guide here: https://chromium.googlesource.com/chromium/src/+/master/docs/testing/writing_... ``` Test files with a single test case should omit the test case description. The file's <title> should be sufficient to describe the scenario being tested. ``` Or do we have other suggestions? Also see: https://groups.google.com/a/chromium.org/d/msg/blink-dev/XsR6PKRrS1E/Guv2rK0b...
https://codereview.chromium.org/2791373002/diff/40001/third_party/WebKit/Layo... File third_party/WebKit/LayoutTests/external/wpt/staticrange/staticrange-immutable.html (right): https://codereview.chromium.org/2791373002/diff/40001/third_party/WebKit/Layo... third_party/WebKit/LayoutTests/external/wpt/staticrange/staticrange-immutable.html:27: }); On 2017/04/11 at 22:11:30, chongz wrote: > On 2017/04/11 21:18:46, qyearsley wrote: > > BTW, is it correct that we're not giving this subtest a name since there's only > > one subtest in this test file and it the test already has a title "StaticRange: > > Attributes are immutable"? > > I was following the guide here: > https://chromium.googlesource.com/chromium/src/+/master/docs/testing/writing_... > ``` > Test files with a single test case should omit the test case description. The file's <title> should be sufficient to describe the scenario being tested. > ``` > > Or do we have other suggestions? > No, that sounds good, I was just curious :-)
https://codereview.chromium.org/2791373002/diff/40001/third_party/WebKit/Layo... File third_party/WebKit/LayoutTests/external/wpt/staticrange/staticrange-immutable.html (right): https://codereview.chromium.org/2791373002/diff/40001/third_party/WebKit/Layo... third_party/WebKit/LayoutTests/external/wpt/staticrange/staticrange-immutable.html:27: }); On 2017/04/11 22:31:50, qyearsley wrote: > On 2017/04/11 at 22:11:30, chongz wrote: > > On 2017/04/11 21:18:46, qyearsley wrote: > > > BTW, is it correct that we're not giving this subtest a name since there's > only > > > one subtest in this test file and it the test already has a title > "StaticRange: > > > Attributes are immutable"? > > > > I was following the guide here: > > > https://chromium.googlesource.com/chromium/src/+/master/docs/testing/writing_... > > ``` > > Test files with a single test case should omit the test case description. The > file's <title> should be sufficient to describe the scenario being tested. > > ``` > > > > Or do we have other suggestions? > > > > No, that sounds good, I was just curious :-) FWIW, I like it this way :)
The CQ bit was checked by chongz@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
> lgtm % constructor, but without the constructor none of the other tests are > possible to write, so resolving > https://github.com/garykac/staticrange/issues/5 > first is a good idea. I've removed constructor and related tests to match current spec. I think we should separate constructor issues from this patch, and probably add it back when there is a resolution. foolip@ does it still look good to you? https://codereview.chromium.org/2791373002/diff/40001/third_party/WebKit/Layo... File third_party/WebKit/LayoutTests/external/wpt/staticrange/StaticRange-constructor.html (right): https://codereview.chromium.org/2791373002/diff/40001/third_party/WebKit/Layo... third_party/WebKit/LayoutTests/external/wpt/staticrange/StaticRange-constructor.html:7: assert_equals(typeof new StaticRange, 'object'); On 2017/04/10 07:37:17, foolip_UTC7 wrote: > The constructor is gone from > http://garykac.github.io/staticrange/index.html#interface-staticrange > > I've filed https://github.com/garykac/staticrange/issues/5 Removed constructor to match spec. We could add it back in another CL when there is a resolution. https://codereview.chromium.org/2791373002/diff/40001/third_party/WebKit/Layo... File third_party/WebKit/LayoutTests/external/wpt/staticrange/idlharness.html (right): https://codereview.chromium.org/2791373002/diff/40001/third_party/WebKit/Layo... third_party/WebKit/LayoutTests/external/wpt/staticrange/idlharness.html:14: [Constructor, On 2017/04/10 07:37:17, foolip_UTC7 wrote: > Match the spec byte-for-byte if possible. (It doesn't have a constructor now.) Removed ``` [Constructor, Exposed=Window] ``` to match spec.
Description was changed from ========== [InputEvent] Make StaticRange immutable and move tests to wpt According to the intent-to-ship in blink-dev there is still ongoing discussion about StaticRange on whether we should make attributes mutable. However this shouldn't block InputEvent and we should ship the immutable version first to keep interop with WebKit. This CL removed |setStart()|, |setEnd()|, |toRange()| and made attributes readonly. See WebKit IDL: https://github.com/WebKit/webkit/blob/1bfaaab3e7bf61dcad32c44a1399914987f9018... Discussion: https://groups.google.com/a/chromium.org/d/msg/blink-dev/W-Q1yWW-zas/UrqnyVAI... BUG=707913 ========== to ========== [InputEvent] Make StaticRange immutable and move tests to wpt According to the intent-to-ship in blink-dev there is still ongoing discussion about StaticRange on whether we should make attributes mutable. However this shouldn't block InputEvent and we should ship the immutable version first to keep interop with WebKit. This CL removed |Constructor()| |setStart()|, |setEnd()|, |toRange()| and made attributes readonly. See WebKit IDL: https://github.com/WebKit/webkit/blob/1bfaaab3e7bf61dcad32c44a1399914987f9018... Discussion: https://groups.google.com/a/chromium.org/d/msg/blink-dev/W-Q1yWW-zas/UrqnyVAI... BUG=707913 ==========
Description was changed from ========== [InputEvent] Make StaticRange immutable and move tests to wpt According to the intent-to-ship in blink-dev there is still ongoing discussion about StaticRange on whether we should make attributes mutable. However this shouldn't block InputEvent and we should ship the immutable version first to keep interop with WebKit. This CL removed |Constructor()| |setStart()|, |setEnd()|, |toRange()| and made attributes readonly. See WebKit IDL: https://github.com/WebKit/webkit/blob/1bfaaab3e7bf61dcad32c44a1399914987f9018... Discussion: https://groups.google.com/a/chromium.org/d/msg/blink-dev/W-Q1yWW-zas/UrqnyVAI... BUG=707913 ========== to ========== [InputEvent] Make StaticRange immutable and move tests to wpt According to the intent-to-ship in blink-dev there is still ongoing discussion about StaticRange on whether we should make attributes mutable. However this shouldn't block InputEvent and we should ship the immutable version first to keep interop with WebKit. This CL removed |Constructor()| |setStart()|, |setEnd()|, |toRange()| and made attributes readonly. See WebKit IDL: https://github.com/WebKit/webkit/blob/1bfaaab3e7bf61dcad32c44a1399914987f9018... StaticRange spec: http://garykac.github.io/staticrange/index.html#interface-staticrange Discussion: https://groups.google.com/a/chromium.org/d/msg/blink-dev/W-Q1yWW-zas/UrqnyVAI... BUG=707913 ==========
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
Not much left of this now, but LGTM :) https://codereview.chromium.org/2791373002/diff/60001/third_party/WebKit/Sour... File third_party/WebKit/Source/core/dom/StaticRange.idl (right): https://codereview.chromium.org/2791373002/diff/60001/third_party/WebKit/Sour... third_party/WebKit/Source/core/dom/StaticRange.idl:16: // TODO(chongz): Add back when the spec becomes stable. Suggest removing this, the spec might not come to match this exactly, and you can copy the spec's IDL when it materializes.
https://codereview.chromium.org/2791373002/diff/60001/third_party/WebKit/Sour... File third_party/WebKit/Source/core/dom/StaticRange.idl (right): https://codereview.chromium.org/2791373002/diff/60001/third_party/WebKit/Sour... third_party/WebKit/Source/core/dom/StaticRange.idl:16: // TODO(chongz): Add back when the spec becomes stable. On 2017/04/13 06:13:40, foolip_UTC7 wrote: > Suggest removing this, the spec might not come to match this exactly, and you > can copy the spec's IDL when it materializes. Done.
The CQ bit was checked by chongz@chromium.org
The patchset sent to the CQ was uploaded after l-g-t-m from qyearsley@chromium.org, foolip@chromium.org Link to the patchset: https://codereview.chromium.org/2791373002/#ps80001 (title: "foolip's comment: Remove TODO")
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Try jobs failed on following builders: linux_chromium_rel_ng on master.tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
The CQ bit was checked by chongz@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
The CQ bit was checked by chongz@chromium.org
The patchset sent to the CQ was uploaded after l-g-t-m from qyearsley@chromium.org, foolip@chromium.org Link to the patchset: https://codereview.chromium.org/2791373002/#ps100001 (title: "Fix global-interface-listing")
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
CQ is committing da patch.
Bot data: {"patchset_id": 100001, "attempt_start_ts": 1492139398814900,
"parent_rev": "426ea5dd2cf7061a7860a34d502f2c191a343a8e", "commit_rev":
"e2b8278dd5aaf5a6ec415aa5e7dd828f17b644f6"}
Message was sent while issue was closed.
Description was changed from ========== [InputEvent] Make StaticRange immutable and move tests to wpt According to the intent-to-ship in blink-dev there is still ongoing discussion about StaticRange on whether we should make attributes mutable. However this shouldn't block InputEvent and we should ship the immutable version first to keep interop with WebKit. This CL removed |Constructor()| |setStart()|, |setEnd()|, |toRange()| and made attributes readonly. See WebKit IDL: https://github.com/WebKit/webkit/blob/1bfaaab3e7bf61dcad32c44a1399914987f9018... StaticRange spec: http://garykac.github.io/staticrange/index.html#interface-staticrange Discussion: https://groups.google.com/a/chromium.org/d/msg/blink-dev/W-Q1yWW-zas/UrqnyVAI... BUG=707913 ========== to ========== [InputEvent] Make StaticRange immutable and move tests to wpt According to the intent-to-ship in blink-dev there is still ongoing discussion about StaticRange on whether we should make attributes mutable. However this shouldn't block InputEvent and we should ship the immutable version first to keep interop with WebKit. This CL removed |Constructor()| |setStart()|, |setEnd()|, |toRange()| and made attributes readonly. See WebKit IDL: https://github.com/WebKit/webkit/blob/1bfaaab3e7bf61dcad32c44a1399914987f9018... StaticRange spec: http://garykac.github.io/staticrange/index.html#interface-staticrange Discussion: https://groups.google.com/a/chromium.org/d/msg/blink-dev/W-Q1yWW-zas/UrqnyVAI... BUG=707913 Review-Url: https://codereview.chromium.org/2791373002 Cr-Commit-Position: refs/heads/master@{#464665} Committed: https://chromium.googlesource.com/chromium/src/+/e2b8278dd5aaf5a6ec415aa5e7dd... ==========
Message was sent while issue was closed.
Committed patchset #5 (id:100001) as https://chromium.googlesource.com/chromium/src/+/e2b8278dd5aaf5a6ec415aa5e7dd... |
