23793 2009-02-06 11:39:32 -0800 WebKit needs separate VisibleSelection and Selection data storage 2023-02-25 07:38:21 -0800 1 1 1 Unclassified WebKit HTML Editing 528+ (Nightly build) Mac OS X 10.5 RESOLVED CONFIGURATION CHANGED P2 Normal --- 23852 6350 17697 1 eric webkit-unassigned ahmad.saleem792 ayg emacemac7 hyatt jparent justin.garcia kai ojan oldest_to_newest 108669 0 eric 2009-02-06 11:39:32 -0800 WebKit changes the Selection on setting with addRange, FF does not This is sorta a dupe of bug 6350 and bug 17697, certainly fixing this more general bug would fix both of those. Basically Selection::validate() is flawed, at least from the DOM's perspective. JS should be able to set the selection to anything and get it back w/o having it be mutated in the process. The Editing code wants to be able to get at the "validated" selection. We may just want to store things double, not sure yet. 108670 1 eric 2009-02-06 11:40:26 -0800 This bug was prompted by http://code.google.com/p/chromium/issues/detail?id=2874, I'm about to attach a modified test case from chromium's 2874 108671 2 27404 eric 2009-02-06 11:41:21 -0800 Created attachment 27404 modified test case from chromium bug 108674 3 eric 2009-02-06 12:01:24 -0800 I think the "validation" invariant which Selection currently tries to hold, is a wrong one. In at least I don't think Selection should assume that both of it's endpoints are valid visible positions. Maybe we need a separate VisibleSelection class which holds that invariant, similar to how we have VisiblePosition and Position split classes. 108675 4 eric 2009-02-06 12:07:00 -0800 In the above proposal, Selection would just be an array of ranges. VisibleSelection would probably still support only one Range (like it does today), slowly editing commands could be transitioned off of VisibleSelection and onto Selection? Maybe adding multiple-range selection support in the process? 108677 5 justin.garcia 2009-02-06 12:24:29 -0800 (In reply to comment #3) > Maybe we need a separate VisibleSelection class which holds that invariant, > similar to how we have VisiblePosition and Position split classes. Yea, that seems like the way to go. 108701 6 eric 2009-02-06 13:27:36 -0800 Ok, so SelectionController will need a VisibleSelection member (the current m_selection), and a new Selection member. What the user sees and interacts with is the VisibleSelection. What JavaScript and (eventually) the editing commands interact with will be Selection (a Vector of Range objects). VisibleSelection and Selection will both be copyable, datastorage types. Editing, and any other section of the code which wants to keep around multiple copies can do so. SelectionController maintain the official copy. :) In order to handle keeping the stored Selection and VisibleSelection in sync, SelectionController will need to change to return a copy of a VisibleSelection via visibleSelection() instead of a reference like it does now. There will be a new setVisibleSelection method which can take a VisibleSelection and knows to set m_selection from VisibleSelection::toSelection() or similar. Likewise there will need to be a setSelection which takes a Selection object and knows to create a VisibleSelection from the Selection and store it in m_visibleSelection. Initially all of our code will use VisibleSelection. Over time, we will move more and more code to use Selection instead. To add multi-range selection (which we'll eventually need for table editing), Selection will just store a Vector of Range objects. VisibleSelection can hold whatever data it needs to represent the selection, but it will probably be a Vector of VisiblePosition pairs (or maybe VisibleRanges?). Thoughts on this proposal are most welcome! 1936752 7 ahmad.saleem792 2023-02-25 07:38:21 -0800 Attached test case also passes in Safari Technology Preview 164 after Live Range Selection API also matching Chrome Canary 112 and Firefox Nightly 112. Marking this as "RESOLVED CONFIGURATION CHANGED". Thanks! 27404 2009-02-06 11:41:21 -0800 2009-02-06 11:41:21 -0800 modified test case from chromium bug caret-set-get.html text/html 3682 eric PCFET0NUWVBFIGh0bWwgUFVCTElDICItLy9XM0MvL0RURCBYSFRNTCAxLjAgU3RyaWN0Ly9FTiIg Imh0dHA6Ly93d3cudzMub3JnL1RSL3hodG1sMS9EVEQveGh0bWwxLXN0cmljdC5kdGQiPgo8aHRt bD4KICA8aGVhZD4KICAgIDx0aXRsZT5TZWxlY3Rpb24gdGVzdDwvdGl0bGU+CiAgICA8c3R5bGU+ CiAgICAgIGRpdiNlZGl0IHsKICAgICAgICBib3JkZXI6MXB4IHNvbGlkIGJsdWU7CiAgICAgICAg cGFkZGluZzo1cHg7CiAgICAgIH0KICAgICAgZGl2I2xvZyB7CiAgICAgICAgbWFyZ2luLXRvcDox MHB4OwogICAgICB9CiAgICAgIC5lcnJvciB7CiAgICAgICAgY29sb3I6cmVkOwogICAgICB9CiAg ICAgIC5zdWNjZXNzIHsKICAgICAgICBjb2xvcjpncmVlbjsKICAgICAgfQogICAgPC9zdHlsZT4K ICAgIDxzY3JpcHQgbGFuZ3VhZ2U9J2phdmFzY3JpcHQnPgoKICAgIHZhciBzdWNjZXNzID0gdHJ1 ZTsKICAgIAogICAgZnVuY3Rpb24gcnVuVGVzdChub2RlKSB7CiAgICAgIGxvZ1RleHQobm9kZS5v dXRlckhUTUwpOwogICAgICB0ZXN0Tm9kZShub2RlKTsKICAgICAgaWYgKHN1Y2Nlc3MpIHsKICAg ICAgICBsb2dTdWNjZXNzKCJQYXNzISIpOwogICAgICB9CiAgICB9CiAgICAgICAgCiAgICBmdW5j dGlvbiBpc1RleHQobm9kZSkgewogICAgICByZXR1cm4gbm9kZS5ub2RlVHlwZSA9PSAzOwogICAg fQogICAgCiAgICBmdW5jdGlvbiB0ZXN0Tm9kZShub2RlKSB7CiAgICAgIGlmIChpc1RleHQobm9k ZSkpIHsKICAgICAgICB0ZXN0VGV4dE5vZGUobm9kZSk7CiAgICAgIH0gZWxzZSB7CiAgICAgICAg dGVzdEVsZW1lbnQobm9kZSk7CiAgICAgIH0KICAgIH0KICAgIAogICAgZnVuY3Rpb24gdGVzdEVs ZW1lbnQoZWxlbWVudCkgewogICAgICB2YXIgY2hpbGRyZW4gPSBlbGVtZW50LmNoaWxkTm9kZXM7 CiAgICAgIGZvciAodmFyIGkgPSAwOyBpIDwgY2hpbGRyZW4ubGVuZ3RoOyBpKyspIHsKICAgICAg ICB0ZXN0Q2FyZXQoZWxlbWVudCwgaSk7CiAgICAgICAgdGVzdE5vZGUoY2hpbGRyZW5baV0pOwog ICAgICB9CiAgICAgIHRlc3RDYXJldChlbGVtZW50LCBjaGlsZHJlbi5sZW5ndGgpOwogICAgfQog ICAgCiAgICBmdW5jdGlvbiB0ZXN0VGV4dE5vZGUodGV4dCkgewogICAgICB2YXIgc3RyaW5nID0g dGV4dC5ub2RlVmFsdWU7CiAgICAgIGZvciAodmFyIGkgPSAwOyBpIDw9IHN0cmluZy5sZW5ndGg7 IGkrKykgewogICAgICAgIHRlc3RDYXJldCh0ZXh0LCBpKTsKICAgICAgfQogICAgfQogICAgCiAg ICBmdW5jdGlvbiBzZXRDYXJldChub2RlLCBvZmZzZXQpIHsKICAgICAgdmFyIGNhcmV0ID0gZG9j dW1lbnQuY3JlYXRlUmFuZ2UoKTsKICAgICAgY2FyZXQuc2V0U3RhcnQobm9kZSwgb2Zmc2V0KTsK ICAgICAgY2FyZXQuc2V0RW5kKG5vZGUsIG9mZnNldCk7CiAgICAgIHZhciBzZWxlY3Rpb24gPSB3 aW5kb3cuZ2V0U2VsZWN0aW9uKCk7CiAgICAgIHNlbGVjdGlvbi5yZW1vdmVBbGxSYW5nZXMoKTsK ICAgICAgc2VsZWN0aW9uLmFkZFJhbmdlKGNhcmV0KTsKICAgIH0KICAgIAogICAgZnVuY3Rpb24g Z2V0Q2FyZXQoKSB7CiAgICAgIHZhciBzZWxlY3Rpb24gPSB3aW5kb3cuZ2V0U2VsZWN0aW9uKCk7 CiAgICAgIHJldHVybiBzZWxlY3Rpb24ucmFuZ2VDb3VudCA+IDAgPyBzZWxlY3Rpb24uZ2V0UmFu Z2VBdCgwKSA6IG51bGw7CiAgICB9CiAgICAKICAgIGZ1bmN0aW9uIGlzQ2FyZXRFcXVhbChub2Rl LCBvZmZzZXQsIGNhcmV0KSB7CiAgICAgIHJldHVybiBjYXJldCAhPSBudWxsICYmIGNhcmV0LmNv bGxhcHNlZCAmJiBub2RlID09IGNhcmV0LnN0YXJ0Q29udGFpbmVyICYmIG9mZnNldCA9PSBjYXJl dC5zdGFydE9mZnNldDsKICAgIH0KCiAgICBmdW5jdGlvbiB0ZXN0Q2FyZXQobm9kZSwgb2Zmc2V0 KSB7CiAgICAgIHNldENhcmV0KG5vZGUsIG9mZnNldCk7CiAgICAgIHZhciBjYXJldCA9IGdldENh cmV0KCk7CiAgICAgIGlmICghaXNDYXJldEVxdWFsKG5vZGUsIG9mZnNldCwgY2FyZXQpKSB7CiAg ICAgICAgc3VjY2VzcyA9IGZhbHNlOwogICAgICAgIGxvZ0NhcmV0RXJyb3Iobm9kZSwgb2Zmc2V0 LCBjYXJldCk7CiAgICAgIH0gZWxzZSB7CiAgICAgICAgICBsb2dDYXJldFN1Y2Nlc3Mobm9kZSwg b2Zmc2V0LCBjYXJldCk7CiAgICAgIH0KICAgIH0KICAgIAogICAgZnVuY3Rpb24gbm9kZTJTdHJp bmcobm9kZSkgewogICAgICBpZiAoaXNUZXh0KG5vZGUpKSB7CiAgICAgICAgcmV0dXJuICInIiAr IG5vZGUubm9kZVZhbHVlICsgIiciOwogICAgICB9IGVsc2UgewogICAgICAgIHZhciBpZCA9IG5v ZGUuaWQ7CiAgICAgICAgcmV0dXJuICImbHQ7IiArIG5vZGUudGFnTmFtZS50b0xvd2VyQ2FzZSgp ICsgKGlkID8gIiMiICsgaWQgOiAiIikgKyAiJmd0OyI7CiAgICAgIH0KICAgIH0KICAgIAogICAg ZnVuY3Rpb24gbG9nSHRtbChodG1sKSB7CiAgICAgIGRvY3VtZW50LmdldEVsZW1lbnRCeUlkKCds b2cnKS5pbm5lckhUTUwgKz0gaHRtbCArICI8YnIvPiI7CiAgICB9CiAgICAKICAgIGZ1bmN0aW9u IGxvZ1RleHQodGV4dCkgewogICAgICB2YXIgc3BhbiA9IGRvY3VtZW50LmNyZWF0ZUVsZW1lbnQo J3NwYW4nKTsKICAgICAgdmFyIHQgPSBkb2N1bWVudC5jcmVhdGVUZXh0Tm9kZSh0ZXh0KTsKICAg ICAgc3Bhbi5hcHBlbmRDaGlsZCh0KTsKICAgICAgZG9jdW1lbnQuZ2V0RWxlbWVudEJ5SWQoJ2xv ZycpLmFwcGVuZENoaWxkKHNwYW4pOwogICAgICBsb2dIdG1sKCIiKTsKICAgIH0KICAgIAogICAg ZnVuY3Rpb24gbG9nQ2FyZXRFcnJvcihub2RlLCBvZmZzZXQsIGNhcmV0KSB7CiAgICAgIHZhciBt c2cgPSAiPHNwYW4gY2xhc3M9J2Vycm9yJz48Yj5TZXQ6PC9iPiAiICsgbm9kZTJTdHJpbmcobm9k ZSkgKyAiIC0gIiArIG9mZnNldDsKICAgICAgbXNnICs9ICIgPGI+R290OjwvYj4gIiArIG5vZGUy U3RyaW5nKGNhcmV0LnN0YXJ0Q29udGFpbmVyKSArICIgLSAiICsgY2FyZXQuc3RhcnRPZmZzZXQ7 CiAgICAgIG1zZyArPSAiPC9zcGFuPiI7CiAgICAgIGxvZ0h0bWwobXNnKTsKICAgIH0KICAgIAog ICAgZnVuY3Rpb24gbG9nQ2FyZXRTdWNjZXNzKG5vZGUsIG9mZnNldCwgY2FyZXQpIHsKICAgICAg dmFyIG1zZyA9ICI8c3BhbiBjbGFzcz0nc3VjY2Vzcyc+PGI+U2V0OjwvYj4gIiArIG5vZGUyU3Ry aW5nKG5vZGUpICsgIiAtICIgKyBvZmZzZXQ7CiAgICAgIG1zZyArPSAiIDxiPkdvdDo8L2I+ICIg KyBub2RlMlN0cmluZyhjYXJldC5zdGFydENvbnRhaW5lcikgKyAiIC0gIiArIGNhcmV0LnN0YXJ0 T2Zmc2V0OwogICAgICBtc2cgKz0gIjwvc3Bhbj4iOwogICAgICBsb2dIdG1sKG1zZyk7CiAgICB9 CiAgICAKICAgIGZ1bmN0aW9uIGxvZ1N1Y2Nlc3MobXNnKSB7CiAgICAgIGxvZ0h0bWwoIjxzcGFu IGNsYXNzPSdzdWNjZXNzJz4iICsgbXNnICsgIjwvc3Bhbj48YnIvPiIpOwogICAgfQogICAgCiAg ICA8L3NjcmlwdD4KICA8L2hlYWQ+CiAgPGJvZHkgb25sb2FkPSJydW5UZXN0KGRvY3VtZW50Lmdl dEVsZW1lbnRCeUlkKCdlZGl0JykpIj4KICAgIDxkaXYgaWQ9ImVkaXQiIGNvbnRlbnRFZGl0YWJs ZT0idHJ1ZSI+c2V0IDxiPnRoZTwvYj4gY2FyZXQgPGEgaHJlZj0iIyI+aGVyZTwvYT48cCBpZD0n Zmlyc3QnPmZvbzwvcD48cCBpZD0nc2Vjb25kJz5iYXI8L3A+PC9kaXY+CiAgICA8ZGl2IGlkPSJs b2ciPgogICAgPC9kaXY+CiAgPC9ib2R5Pgo8L2h0bWw+Cg==