138494 2014-11-06 22:08:15 -0800 collapse api in DOMSelection IDL does not match as per specification 2022-08-11 14:06:06 -0700 1 1 1 Unclassified WebKit DOM 528+ (Nightly build) Unspecified Unspecified NEW P2 Normal --- 1 shiva.jm webkit-unassigned ahmad.saleem792 ap bfulgham buildbot cdumez darin gyuyoung.kim rniwa simon.fraser oldest_to_newest 1047026 0 shiva.jm 2014-11-06 22:08:15 -0800 collapse api in DOMSelection IDL does not match as per specification: http://www.w3.org/TR/selection-api/#widl-Selection-collapse-void-Node-node-unsigned-long-offset. Specificaton does not mandate Node and offset to be optional, also node should not null, which is already covered in DOMSelection::isValidForPosition() 1047036 1 241165 shiva.jm 2014-11-07 00:34:22 -0800 Created attachment 241165 Patch collapse api in DOMSelection IDL does not match as per specification 1047205 2 241165 darin 2014-11-07 15:23:09 -0800 Comment on attachment 241165 Patch View in context: https://bugs.webkit.org/attachment.cgi?id=241165&action=review Good change, but requires a test. > Source/WebCore/ChangeLog:8 > + No new tests, no behavior change. That is not true. This is a behavior change. This change needs test coverage. 1047225 3 241165 rniwa 2014-11-07 15:42:27 -0800 Comment on attachment 241165 Patch View in context: https://bugs.webkit.org/attachment.cgi?id=241165&action=review > Source/WebCore/page/DOMSelection.idl:44 > - [RaisesException] void collapse([Default=Undefined] optional Node node, > - [Default=Undefined] optional long index); > + [RaisesException] void collapse(Node node, long index); I'm not certain if this is a backwards compatible change. It may break contents that rely on these arguments to be optional in WebKit. 1047793 4 241341 shiva.jm 2014-11-11 00:40:55 -0800 Created attachment 241341 Patch-Updated-Review1 Updated the patch with tests. 1047879 5 241341 cdumez 2014-11-11 09:59:20 -0800 Comment on attachment 241341 Patch-Updated-Review1 View in context: https://bugs.webkit.org/attachment.cgi?id=241341&action=review > Source/WebCore/ChangeLog:10 > + Chrome 38. How about Firefox and IE? > Source/WebCore/page/DOMSelection.idl:44 > + [RaisesException] void collapse(Node node, [Default=Undefined] optional long index); Please make sure calling collapse(null) still works (clears selection) as content apparently relies on this: https://codereview.chromium.org/546703003/ We should add test coverage for this as well. 1047880 6 241341 cdumez 2014-11-11 10:08:16 -0800 Comment on attachment 241341 Patch-Updated-Review1 r- because the patch seems to break several layout tests (yes, our layout tests call collapse() without argument). 1048080 7 shiva.jm 2014-11-11 19:33:59 -0800 (In reply to comment #6) > Comment on attachment 241341 [details] > Patch-Updated-Review1 > > r- because the patch seems to break several layout tests (yes, our layout > tests call collapse() without argument). (In reply to comment #6) > Comment on attachment 241341 [details] > Patch-Updated-Review1 > > r- because the patch seems to break several layout tests (yes, our layout > tests call collapse() without argument). Dear Chris, Thanks for inputs, will modify all tests, also make selection content clears and add test coverage for the same. 1890696 8 ahmad.saleem792 2022-08-11 14:05:52 -0700 I took the test case from the patch and changed it into JSFiddle: Link - https://jsfiddle.net/xs17np58/show and these are results across all browsers: *** Safari 15.6 on macOS 12.5 *** PASS selection.rangeCount is 1 PASS selection.collapse() threw exception TypeError: Not enough arguments. PASS selection.rangeCount is 1 PASS selection.rangeCount is 1 PASS selection.rangeCount is 0 FAIL successfullyParsed should be true. Was false. Some tests failed. TEST COMPLETE *** Chrome Canary 105 *** PASS selection.rangeCount is 1 FAIL selection.collapse() should throw TypeError: Not enough arguments. Threw exception TypeError: Failed to execute 'collapse' on 'Selection': 1 argument required, but only 0 present.. PASS selection.rangeCount is 1 PASS selection.rangeCount is 1 PASS selection.rangeCount is 0 FAIL successfullyParsed should be true. Was false. Some tests failed. *** Firefox Nightly 105 *** PASS selection.rangeCount is 1 FAIL selection.collapse() should throw TypeError: Not enough arguments. Threw exception TypeError: Selection.collapse: At least 1 argument required, but only 0 passed. PASS selection.rangeCount is 1 PASS selection.rangeCount is 1 PASS selection.rangeCount is 0 FAIL successfullyParsed should be true. Was false. Some tests failed. TEST COMPLETE _______________ As can be seen from the test results, all fail on "successfulyParsed" but Chrome Canary 106 and Firefox Nightly also fail on "selection.collapse()" compared to Safari, I don't know whether it is intentional to align with web-spec or not but just wanted to share updated status. Please mark this bug accordingly. Thank! 241165 2014-11-07 00:34:22 -0800 2014-11-11 00:40:55 -0800 Patch bug-138494.patch text/plain 1259 shiva.jm SW5kZXg6IFNvdXJjZS9XZWJDb3JlL0NoYW5nZUxvZwo9PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09Ci0tLSBTb3VyY2UvV2Vi Q29yZS9DaGFuZ2VMb2cJKHJldmlzaW9uIDE3NTc0NCkKKysrIFNvdXJjZS9XZWJDb3JlL0NoYW5n ZUxvZwkod29ya2luZyBjb3B5KQpAQCAtMSwzICsxLDE0IEBACisyMDE0LTExLTA3ICBTaGl2YWt1 bWFyIEpNICA8c2hpdmEuam1Ac2Ftc3VuZy5jb20+CisKKyAgICAgICAgY29sbGFwc2UgYXBpIGlu IERPTVNlbGVjdGlvbiBJREwgZG9lcyBub3QgbWF0Y2ggYXMgcGVyIHNwZWNpZmljYXRpb24KKyAg ICAgICAgaHR0cHM6Ly9idWdzLndlYmtpdC5vcmcvc2hvd19idWcuY2dpP2lkPTEzODQ5NAorCisg ICAgICAgIFJldmlld2VkIGJ5IE5PQk9EWSAoT09QUyEpLgorCisgICAgICAgIE5vIG5ldyB0ZXN0 cywgbm8gYmVoYXZpb3IgY2hhbmdlLgorCisgICAgICAgICogcGFnZS9ET01TZWxlY3Rpb24uaWRs OgorCiAyMDE0LTExLTA2ICBDYXJsb3MgR2FyY2lhIENhbXBvcyAgPGNnYXJjaWFAaWdhbGlhLmNv bT4KIAogICAgICAgICBVbnJldmlld2VkLiBUcnkgdG8gZml4IEdUSyBidWlsZCBhZnRlciByMTc1 NzE5LgpJbmRleDogU291cmNlL1dlYkNvcmUvcGFnZS9ET01TZWxlY3Rpb24uaWRsCj09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT0KLS0tIFNvdXJjZS9XZWJDb3JlL3BhZ2UvRE9NU2VsZWN0aW9uLmlkbAkocmV2aXNpb24gMTc1 NzQzKQorKysgU291cmNlL1dlYkNvcmUvcGFnZS9ET01TZWxlY3Rpb24uaWRsCSh3b3JraW5nIGNv cHkpCkBAIC00MSw4ICs0MSw3IEBACiAgICAgcmVhZG9ubHkgYXR0cmlidXRlIGJvb2xlYW4gaXND b2xsYXBzZWQ7CiAgICAgcmVhZG9ubHkgYXR0cmlidXRlIGxvbmcgcmFuZ2VDb3VudDsKIAotICAg IFtSYWlzZXNFeGNlcHRpb25dIHZvaWQgY29sbGFwc2UoW0RlZmF1bHQ9VW5kZWZpbmVkXSBvcHRp b25hbCBOb2RlIG5vZGUsIAotICAgICAgICAgICAgICAgICAgW0RlZmF1bHQ9VW5kZWZpbmVkXSBv cHRpb25hbCBsb25nIGluZGV4KTsKKyAgICBbUmFpc2VzRXhjZXB0aW9uXSB2b2lkIGNvbGxhcHNl KE5vZGUgbm9kZSwgbG9uZyBpbmRleCk7CiAgICAgW1JhaXNlc0V4Y2VwdGlvbl0gdm9pZCBjb2xs YXBzZVRvRW5kKCk7CiAgICAgW1JhaXNlc0V4Y2VwdGlvbl0gdm9pZCBjb2xsYXBzZVRvU3RhcnQo KTsKIAo= 241341 2014-11-11 00:40:55 -0800 2014-11-11 10:08:16 -0800 Patch-Updated-Review1 bug-138494-updated.patch text/plain 4325 shiva.jm SW5kZXg6IFNvdXJjZS9XZWJDb3JlL0NoYW5nZUxvZwo9PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09Ci0tLSBTb3VyY2UvV2Vi Q29yZS9DaGFuZ2VMb2cJKHJldmlzaW9uIDE3NTg2MSkKKysrIFNvdXJjZS9XZWJDb3JlL0NoYW5n ZUxvZwkod29ya2luZyBjb3B5KQpAQCAtMSwzICsxLDE4IEBACisyMDE0LTExLTExICBTaGl2YWt1 bWFyIEpNICA8c2hpdmEuam1Ac2Ftc3VuZy5jb20+CisKKyAgICAgICAgY29sbGFwc2UgYXBpIGlu IERPTVNlbGVjdGlvbiBJREwgZG9lcyBub3QgbWF0Y2ggYXMgcGVyIHNwZWNpZmljYXRpb24uCisg ICAgICAgIGh0dHBzOi8vYnVncy53ZWJraXQub3JnL3Nob3dfYnVnLmNnaT9pZD0xMzg0OTQKKwor ICAgICAgICBSZXZpZXdlZCBieSBOT0JPRFkgKE9PUFMhKS4KKworICAgICAgICBNYWtlIHRoZSBO b2RlIGFyZ3VtZW50IGZvciBjb2xsYXBzZSBBUEkgYXMgbWFuZGF0b3J5IGFzIHBlciBzcGVjaWZp Y2F0aW9uOiBodHRwOi8vd3d3LnczLm9yZy9UUi9zZWxlY3Rpb24tYXBpLyN3aWRsLVNlbGVjdGlv bi1jb2xsYXBzZS0KKyAgICAgICAgdm9pZC1Ob2RlLW5vZGUtdW5zaWduZWQtbG9uZy1vZmZzZXQu IFRvIG1ha2UgYmFja3dhcmQgY29tcGF0aWJsZSBrZWVwIGluZGV4IGFyZ3VtZW50IGFzIG9wdGlv bmFsLiBBbHNvIFRoaXMgbWF0Y2hlcyB0aGUgYmVoYXZpb3Igb2YKKyAgICAgICAgQ2hyb21lIDM4 LgorCisgICAgICAgIFRlc3Q6IGVkaXRpbmcvc2VsZWN0aW9uL3NlbGVjdGlvbi1jb2xsYXBzZS5o dG1sCisKKyAgICAgICAgKiBwYWdlL0RPTVNlbGVjdGlvbi5pZGw6CisKIDIwMTQtMTEtMTAgIENo cmlzIER1bWV6ICA8Y2R1bWV6QGFwcGxlLmNvbT4KIAogICAgICAgICBMYXppbHkgY3JlYXRlIEhU TUxJbnB1dEVsZW1lbnQncyBpbnB1dFR5cGUgYW5kIHNoYWRvdyBzdWJ0cmVlCkluZGV4OiBTb3Vy Y2UvV2ViQ29yZS9wYWdlL0RPTVNlbGVjdGlvbi5pZGwKPT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PQotLS0gU291cmNlL1dl YkNvcmUvcGFnZS9ET01TZWxlY3Rpb24uaWRsCShyZXZpc2lvbiAxNzU4NjEpCisrKyBTb3VyY2Uv V2ViQ29yZS9wYWdlL0RPTVNlbGVjdGlvbi5pZGwJKHdvcmtpbmcgY29weSkKQEAgLTQxLDggKzQx LDcgQEAKICAgICByZWFkb25seSBhdHRyaWJ1dGUgYm9vbGVhbiBpc0NvbGxhcHNlZDsKICAgICBy ZWFkb25seSBhdHRyaWJ1dGUgbG9uZyByYW5nZUNvdW50OwogCi0gICAgW1JhaXNlc0V4Y2VwdGlv bl0gdm9pZCBjb2xsYXBzZShbRGVmYXVsdD1VbmRlZmluZWRdIG9wdGlvbmFsIE5vZGUgbm9kZSwg Ci0gICAgICAgICAgICAgICAgICBbRGVmYXVsdD1VbmRlZmluZWRdIG9wdGlvbmFsIGxvbmcgaW5k ZXgpOworICAgIFtSYWlzZXNFeGNlcHRpb25dIHZvaWQgY29sbGFwc2UoTm9kZSBub2RlLCBbRGVm YXVsdD1VbmRlZmluZWRdIG9wdGlvbmFsIGxvbmcgaW5kZXgpOwogICAgIFtSYWlzZXNFeGNlcHRp b25dIHZvaWQgY29sbGFwc2VUb0VuZCgpOwogICAgIFtSYWlzZXNFeGNlcHRpb25dIHZvaWQgY29s bGFwc2VUb1N0YXJ0KCk7CiAKSW5kZXg6IExheW91dFRlc3RzL0NoYW5nZUxvZwo9PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 Ci0tLSBMYXlvdXRUZXN0cy9DaGFuZ2VMb2cJKHJldmlzaW9uIDE3NTg2MSkKKysrIExheW91dFRl c3RzL0NoYW5nZUxvZwkod29ya2luZyBjb3B5KQpAQCAtMSwzICsxLDEzIEBACisyMDE0LTExLTEx ICBTaGl2YWt1bWFyIEpNICA8c2hpdmEuam1Ac2Ftc3VuZy5jb20+CisKKyAgICAgICAgY29sbGFw c2UgYXBpIGluIERPTVNlbGVjdGlvbiBJREwgZG9lcyBub3QgbWF0Y2ggYXMgcGVyIHNwZWNpZmlj YXRpb24uCisgICAgICAgIGh0dHBzOi8vYnVncy53ZWJraXQub3JnL3Nob3dfYnVnLmNnaT9pZD0x Mzg0OTQKKworICAgICAgICBSZXZpZXdlZCBieSBOT0JPRFkgKE9PUFMhKS4KKworICAgICAgICAq IGVkaXRpbmcvc2VsZWN0aW9uL3NlbGVjdGlvbi1jb2xsYXBzZS1leHBlY3RlZC50eHQ6IEFkZGVk LgorICAgICAgICAqIGVkaXRpbmcvc2VsZWN0aW9uL3NlbGVjdGlvbi1jb2xsYXBzZS5odG1sOiBB ZGRlZC4KKwogMjAxNC0xMS0xMCAgQmVuamFtaW4gUG91bGFpbiAgPGJwb3VsYWluQGFwcGxlLmNv bT4KIAogICAgICAgICBBZGQgcGFyc2luZyBzdXBwb3J0IGZvciB0aGUgZXh0ZW5kZWQgOm50aC1s YXN0LWNoaWxkKEFuK0Igb2Ygc2VsZWN0b3ItbGlzdCkKSW5kZXg6IExheW91dFRlc3RzL2VkaXRp bmcvc2VsZWN0aW9uL3NlbGVjdGlvbi1jb2xsYXBzZS1leHBlY3RlZC50eHQKPT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PQot LS0gTGF5b3V0VGVzdHMvZWRpdGluZy9zZWxlY3Rpb24vc2VsZWN0aW9uLWNvbGxhcHNlLWV4cGVj dGVkLnR4dAkocmV2aXNpb24gMCkKKysrIExheW91dFRlc3RzL2VkaXRpbmcvc2VsZWN0aW9uL3Nl bGVjdGlvbi1jb2xsYXBzZS1leHBlY3RlZC50eHQJKHJldmlzaW9uIDApCkBAIC0wLDAgKzEsMTcg QEAKK1Rlc3QgZm9yIGNvbGxhcHNlIGFwaSBwYXJhbWV0ZXJzCisKK09uIHN1Y2Nlc3MsIHlvdSB3 aWxsIHNlZSBhIHNlcmllcyBvZiAiUEFTUyIgbWVzc2FnZXMsIGZvbGxvd2VkIGJ5ICJURVNUIENP TVBMRVRFIi4KKworCitQQVNTIHNlbGVjdGlvbi5yYW5nZUNvdW50IGlzIDEKK1BBU1Mgc2VsZWN0 aW9uLmNvbGxhcHNlKCkgdGhyZXcgZXhjZXB0aW9uIFR5cGVFcnJvcjogTm90IGVub3VnaCBhcmd1 bWVudHMuCitQQVNTIHNlbGVjdGlvbi5yYW5nZUNvdW50IGlzIDEKK1BBU1Mgc2VsZWN0aW9uLnJh bmdlQ291bnQgaXMgMQorUEFTUyBzZWxlY3Rpb24ucmFuZ2VDb3VudCBpcyAwCitQQVNTIHNlbGVj dGlvbi5yYW5nZUNvdW50IGlzIDAKK1BBU1Mgc2VsZWN0aW9uLmNvbGxhcHNlKGRpdi5maXJzdENo aWxkLCAtMSkgdGhyZXcgZXhjZXB0aW9uIEVycm9yOiBJbmRleFNpemVFcnJvcjogRE9NIEV4Y2Vw dGlvbiAxLgorUEFTUyBzZWxlY3Rpb24ucmFuZ2VDb3VudCBpcyAxCitQQVNTIHN1Y2Nlc3NmdWxs eVBhcnNlZCBpcyB0cnVlCisKK1RFU1QgQ09NUExFVEUKK0hlbGxvIFdvcmxkCkluZGV4OiBMYXlv dXRUZXN0cy9lZGl0aW5nL3NlbGVjdGlvbi9zZWxlY3Rpb24tY29sbGFwc2UuaHRtbAo9PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09Ci0tLSBMYXlvdXRUZXN0cy9lZGl0aW5nL3NlbGVjdGlvbi9zZWxlY3Rpb24tY29sbGFwc2Uu aHRtbAkocmV2aXNpb24gMCkKKysrIExheW91dFRlc3RzL2VkaXRpbmcvc2VsZWN0aW9uL3NlbGVj dGlvbi1jb2xsYXBzZS5odG1sCShyZXZpc2lvbiAwKQpAQCAtMCwwICsxLDI4IEBACis8IURPQ1RZ UEUgSFRNTD4KKzxodG1sPgorPGJvZHk+Cis8c2NyaXB0IHNyYz0iLi4vLi4vcmVzb3VyY2VzL2pz LXRlc3QuanMiPjwvc2NyaXB0PgorPGRpdiBpZD0iZGl2IiBjb250ZW50ZWRpdGFibGU9InRydWUi PkhlbGxvIFdvcmxkPC9kaXY+Cis8c2NyaXB0PgorCitkZXNjcmlwdGlvbigiVGVzdCBmb3IgY29s bGFwc2UgYXBpIHBhcmFtZXRlcnMiKTsKKwordmFyIHNlbGVjdGlvbiA9IGdldFNlbGVjdGlvbigp Oworc2VsZWN0aW9uLnNlbGVjdEFsbENoaWxkcmVuKGRpdik7CisKK3Nob3VsZEJlKCJzZWxlY3Rp b24ucmFuZ2VDb3VudCIsICIxIik7CitzaG91bGRUaHJvdygic2VsZWN0aW9uLmNvbGxhcHNlKCki LCAiJ1R5cGVFcnJvcjogTm90IGVub3VnaCBhcmd1bWVudHMnIik7CitzaG91bGRCZSgic2VsZWN0 aW9uLnJhbmdlQ291bnQiLCAiMSIpOworc2VsZWN0aW9uLmNvbGxhcHNlKGRpdi5maXJzdENoaWxk LCAyKTsKK3Nob3VsZEJlKCJzZWxlY3Rpb24ucmFuZ2VDb3VudCIsICIxIik7CitzZWxlY3Rpb24u Y29sbGFwc2UobnVsbCk7CitzaG91bGRCZSgic2VsZWN0aW9uLnJhbmdlQ291bnQiLCAiMCIpOwor c2VsZWN0aW9uLmNvbGxhcHNlKDEpOworc2hvdWxkQmUoInNlbGVjdGlvbi5yYW5nZUNvdW50Iiwg IjAiKTsKK3Nob3VsZFRocm93KCJzZWxlY3Rpb24uY29sbGFwc2UoZGl2LmZpcnN0Q2hpbGQsIC0x KSIsICciRXJyb3I6IEluZGV4U2l6ZUVycm9yOiBET00gRXhjZXB0aW9uIDEiJyk7CitzZWxlY3Rp b24uY29sbGFwc2UoZGl2LmZpcnN0Q2hpbGQsIDEwMDAwKTsKK3Nob3VsZEJlKCJzZWxlY3Rpb24u cmFuZ2VDb3VudCIsICIxIik7CisKKzwvc2NyaXB0PgorPC9ib2R5PgorPC9odG1sPgo=