42841 2010-07-22 12:04:35 -0700 [cleanup] logic to extract adjacent lists and list children in listifyParagraph and unlistifyParagraph should be isolated 2010-07-22 19:27:52 -0700 1 1 1 Unclassified WebKit HTML Editing 528+ (Nightly build) All All RESOLVED FIXED P2 Blocker --- 33668 1 rniwa rniwa enrica eric justin.garcia ojan tkent tony oldest_to_newest 254914 0 rniwa 2010-07-22 12:04:35 -0700 To simplify the patch for the bug 33668, we have to extract the code to find adjacent lists in listifyParagraph and list children in unlistifyParagraph. 254922 1 62326 rniwa 2010-07-22 12:28:49 -0700 Created attachment 62326 cleanup patch 255136 2 62326 tkent 2010-07-22 18:34:32 -0700 Comment on attachment 62326 cleanup patch > @@ -268,8 +281,6 @@ PassRefPtr<HTMLElement> InsertListComman > // Update the start of content, so we don't try to move the list into itself. bug 19066 > if (insertionPos == start.deepEquivalent()) > start = startOfParagraph(originalStart); > - previousList = outermostEnclosingList(previousPosition.deepEquivalent().node(), enclosingList(listElement.get())); > - nextList = outermostEnclosingList(nextPosition.deepEquivalent().node(), enclosingList(listElement.get())); > } > > moveParagraph(start, end, VisiblePosition(Position(placeholder.get(), 0)), true); > @@ -281,6 +292,8 @@ PassRefPtr<HTMLElement> InsertListComman > listElement = m_listElement; > > if (listElement) { > + previousList = listElement->previousElementSibling(); > + nextList = listElement->nextElementSibling(); > if (canMergeLists(previousList, listElement.get())) > mergeIdenticalElements(previousList, listElement.get()); > if (canMergeLists(listElement.get(), nextList)) I don't understand why these changes for previousList/nextList are correct. Would you explain it please? 255148 3 rniwa 2010-07-22 18:52:06 -0700 (In reply to comment #2) > (From update of attachment 62326 [details]) > > @@ -268,8 +281,6 @@ PassRefPtr<HTMLElement> InsertListComman > > // Update the start of content, so we don't try to move the list into itself. bug 19066 > > if (insertionPos == start.deepEquivalent()) > > start = startOfParagraph(originalStart); > > - previousList = outermostEnclosingList(previousPosition.deepEquivalent().node(), enclosingList(listElement.get())); > > - nextList = outermostEnclosingList(nextPosition.deepEquivalent().node(), enclosingList(listElement.get())); > > } > > > > moveParagraph(start, end, VisiblePosition(Position(placeholder.get(), 0)), true); > > @@ -281,6 +292,8 @@ PassRefPtr<HTMLElement> InsertListComman > > listElement = m_listElement; > > > > if (listElement) { > > + previousList = listElement->previousElementSibling(); > > + nextList = listElement->nextElementSibling(); > > if (canMergeLists(previousList, listElement.get())) > > mergeIdenticalElements(previousList, listElement.get()); > > if (canMergeLists(listElement.get(), nextList)) > > I don't understand why these changes for previousList/nextList are correct. Would you explain it please? If we're inserting a list element, then previousList must be the previous element sibling of the list element we've just inserted because that's the only case we can merge the two lists. Notice that the only reason we're obtaining previous and next lists are to merge them with the list we've inserted. But we can't merge the lists unless they're siblings of each other since if had some other elements in between, we shouldn't be deleting that element. The argument for nextList is similar. While the original code may obtain elements that are not siblings of listElement, conditions in canMergeLists prevented such elements to be merged with listElement. 255152 4 62326 tkent 2010-07-22 18:58:00 -0700 Comment on attachment 62326 cleanup patch Ok. 255164 5 rniwa 2010-07-22 19:27:52 -0700 Landed as http://trac.webkit.org/changeset/63941. 62326 2010-07-22 12:28:49 -0700 2010-07-22 18:58:00 -0700 cleanup patch fix42841 text/plain 6698 rniwa SW5kZXg6IFdlYkNvcmUvQ2hhbmdlTG9nCj09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT0KLS0tIFdlYkNvcmUvQ2hhbmdlTG9n CShyZXZpc2lvbiA2MzkwNSkKKysrIFdlYkNvcmUvQ2hhbmdlTG9nCSh3b3JraW5nIGNvcHkpCkBA IC0xLDMgKzEsMjUgQEAKKzIwMTAtMDctMjIgIFJ5b3N1a2UgTml3YSAgPHJuaXdhQHdlYmtpdC5v cmc+CisKKyAgICAgICAgUmV2aWV3ZWQgYnkgTk9CT0RZIChPT1BTISkuCisKKyAgICAgICAgW2Ns ZWFudXBdIGxvZ2ljIHRvIGV4dHJhY3QgYWRqYWNlbnQgbGlzdHMgYW5kIGxpc3QgY2hpbGRyZW4g aW4gbGlzdGlmeVBhcmFncmFwaCBhbmQgdW5saXN0aWZ5UGFyYWdyYXBoIHNob3VsZCBiZSBpc29s YXRlZAorICAgICAgICBodHRwczovL2J1Z3Mud2Via2l0Lm9yZy9zaG93X2J1Zy5jZ2k/aWQ9NDI4 NDEKKworICAgICAgICBBZGRlZCBlbmNsb3NpbmdMaXN0Q2hpbGQgdGhhdCB0YWtlcyB0aGUgbGlz dCBub2RlIGNvbnRhaW5pbmcgdGhlIG5vZGUgZm9yIHRoZSBzZWNvbmQgYXJndW1lbnQKKyAgICAg ICAgdG8gZW5zdXJlIHRoYXQgaXQgb25seSByZXR1cm5zIHRoZSBlbmNsb3NpbmcgbGlzdCBjaGls ZCBvZiB0aGUgc3BlY2lmaWVkIGxpc3Qgbm9kZS4KKworICAgICAgICBBZGRlZCBhZGphY2VudEVu Y2xvc2luZ0xpc3QgdGhhdCBmaW5kcyBhZGphY2VudCBsaXN0cyBvZiB0aGUgc3BlY2lmaWVkIHBv c2l0aW9uLgorICAgICAgICBJdCB0YWtlcyB0aGUgY3VycmVudCBwb3NpdGlvbiwgdGhlIGFkamFj ZW50IHBvc2l0aW9uIHRoYXQgbWF5IG9yIG1heSBub3QgaGF2ZSBhIGxpc3QsIGFuZCB0aGUgbGlz dCB0eXBlLAorICAgICAgICBhbmQgYXZvaWRzIGEgbGlzdCB0aGF0IGJlbG9uZ3MgdG8gYSBkaWZm ZXJlbnQgdGFibGUgY2VsbCBvciBhbiBvdXRlciBsaXN0IHRoYXQgY29udGFpbnMgdGhlIGN1cnJl bnQgcG9zaXRpb24uCisKKyAgICAgICAgTm8gbmV3IHRlc3RzIG5lZWRlZC4KKworICAgICAgICAq IGVkaXRpbmcvSW5zZXJ0TGlzdENvbW1hbmQuY3BwOgorICAgICAgICAoV2ViQ29yZTo6ZW5jbG9z aW5nTGlzdENoaWxkKTogQWRkZWQuIFNlZSBhYm92ZS4KKyAgICAgICAgKFdlYkNvcmU6Okluc2Vy dExpc3RDb21tYW5kOjp1bmxpc3RpZnlQYXJhZ3JhcGgpOiBVc2VzIGVuY2xvc2luZ0xpc3RDaGls ZC4KKyAgICAgICAgKFdlYkNvcmU6OmFkamFjZW50RW5jbG9zaW5nTGlzdCk6IEFkZGVkLiBTZWUg YWJvdmUuCisgICAgICAgIChXZWJDb3JlOjpJbnNlcnRMaXN0Q29tbWFuZDo6bGlzdGlmeVBhcmFn cmFwaCk6IFVzZXMgYWRqYWNlbnRFbmNsb3NpbmdMaXN0LgorCiAyMDEwLTA3LTIyICBTYW0gV2Vp bmlnICA8c2FtQHdlYmtpdC5vcmc+CiAKICAgICAgICAgUmV2aWV3ZWQgYnkgQW5kZXJzIENhcmxz c29uLgpJbmRleDogV2ViQ29yZS9lZGl0aW5nL0luc2VydExpc3RDb21tYW5kLmNwcAo9PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09Ci0tLSBXZWJDb3JlL2VkaXRpbmcvSW5zZXJ0TGlzdENvbW1hbmQuY3BwCShyZXZpc2lvbiA2 Mzg3MykKKysrIFdlYkNvcmUvZWRpdGluZy9JbnNlcnRMaXN0Q29tbWFuZC5jcHAJKHdvcmtpbmcg Y29weSkKQEAgLTE1MCw2ICsxNTAsMTQgQEAgdm9pZCBJbnNlcnRMaXN0Q29tbWFuZDo6ZG9BcHBs eUZvclNpbmdsZQogICAgICAgICBtX2xpc3RFbGVtZW50ID0gbGlzdGlmeVBhcmFncmFwaChlbmRp bmdTZWxlY3Rpb24oKS52aXNpYmxlU3RhcnQoKSwgbGlzdFRhZyk7CiB9CiAKK3N0YXRpYyBOb2Rl KiBlbmNsb3NpbmdMaXN0Q2hpbGQoTm9kZSogbm9kZSwgTm9kZSogbGlzdE5vZGUpCit7CisgICAg Tm9kZSogbGlzdENoaWxkID0gZW5jbG9zaW5nTGlzdENoaWxkKG5vZGUpOworICAgIGlmIChlbmNs b3NpbmdMaXN0KGxpc3RDaGlsZCkgIT0gbGlzdE5vZGUpCisgICAgICAgIHJldHVybiAwOworICAg IHJldHVybiBsaXN0Q2hpbGQ7Cit9CisKIHZvaWQgSW5zZXJ0TGlzdENvbW1hbmQ6OnVubGlzdGlm eVBhcmFncmFwaChjb25zdCBWaXNpYmxlUG9zaXRpb24mIG9yaWdpbmFsU3RhcnQsIEhUTUxFbGVt ZW50KiBsaXN0Tm9kZSwgTm9kZSogbGlzdENoaWxkTm9kZSkKIHsKICAgICBOb2RlKiBuZXh0TGlz dENoaWxkOwpAQCAtMTY1LDE0ICsxNzMsMTAgQEAgdm9pZCBJbnNlcnRMaXN0Q29tbWFuZDo6dW5s aXN0aWZ5UGFyYWdyYQogICAgICAgICAvLyBBIHBhcmFncmFwaCBpcyB2aXN1YWxseSBhIGxpc3Qg aXRlbSBtaW51cyBhIGxpc3QgbWFya2VyLiAgVGhlIHBhcmFncmFwaCB3aWxsIGJlIG1vdmVkLgog ICAgICAgICBzdGFydCA9IHN0YXJ0T2ZQYXJhZ3JhcGgob3JpZ2luYWxTdGFydCk7CiAgICAgICAg IGVuZCA9IGVuZE9mUGFyYWdyYXBoKHN0YXJ0KTsKLSAgICAgICAgbmV4dExpc3RDaGlsZCA9IGVu Y2xvc2luZ0xpc3RDaGlsZChlbmQubmV4dCgpLmRlZXBFcXVpdmFsZW50KCkubm9kZSgpKTsKKyAg ICAgICAgbmV4dExpc3RDaGlsZCA9IGVuY2xvc2luZ0xpc3RDaGlsZChlbmQubmV4dCgpLmRlZXBF cXVpdmFsZW50KCkubm9kZSgpLCBsaXN0Tm9kZSk7CiAgICAgICAgIEFTU0VSVChuZXh0TGlzdENo aWxkICE9IGxpc3RDaGlsZE5vZGUpOwotICAgICAgICBpZiAoZW5jbG9zaW5nTGlzdChuZXh0TGlz dENoaWxkKSAhPSBsaXN0Tm9kZSkKLSAgICAgICAgICAgIG5leHRMaXN0Q2hpbGQgPSAwOwotICAg ICAgICBwcmV2aW91c0xpc3RDaGlsZCA9IGVuY2xvc2luZ0xpc3RDaGlsZChzdGFydC5wcmV2aW91 cygpLmRlZXBFcXVpdmFsZW50KCkubm9kZSgpKTsKKyAgICAgICAgcHJldmlvdXNMaXN0Q2hpbGQg PSBlbmNsb3NpbmdMaXN0Q2hpbGQoc3RhcnQucHJldmlvdXMoKS5kZWVwRXF1aXZhbGVudCgpLm5v ZGUoKSwgbGlzdE5vZGUpOwogICAgICAgICBBU1NFUlQocHJldmlvdXNMaXN0Q2hpbGQgIT0gbGlz dENoaWxkTm9kZSk7Ci0gICAgICAgIGlmIChlbmNsb3NpbmdMaXN0KHByZXZpb3VzTGlzdENoaWxk KSAhPSBsaXN0Tm9kZSkKLSAgICAgICAgICAgIHByZXZpb3VzTGlzdENoaWxkID0gMDsKICAgICB9 CiAgICAgLy8gV2hlbiByZW1vdmluZyBhIGxpc3QsIHdlIG11c3QgYWx3YXlzIGNyZWF0ZSBhIHBs YWNlaG9sZGVyIHRvIGFjdCBhcyBhIHBvaW50IG9mIGluc2VydGlvbgogICAgIC8vIGZvciB0aGUg bGlzdCBjb250ZW50IGJlaW5nIHJlbW92ZWQuCkBAIC0yMTAsMjggKzIxNCwzNyBAQCB2b2lkIElu c2VydExpc3RDb21tYW5kOjp1bmxpc3RpZnlQYXJhZ3JhCiAgICAgbW92ZVBhcmFncmFwaHMoc3Rh cnQsIGVuZCwgaW5zZXJ0aW9uUG9pbnQsIHRydWUpOwogfQogCitzdGF0aWMgRWxlbWVudCogYWRq YWNlbnRFbmNsb3NpbmdMaXN0KGNvbnN0IFZpc2libGVQb3NpdGlvbiYgcG9zLCBjb25zdCBWaXNp YmxlUG9zaXRpb24mIGFkamFjZW50UG9zLCBjb25zdCBRdWFsaWZpZWROYW1lJiBsaXN0VGFnKQor eworICAgIEVsZW1lbnQqIGxpc3ROb2RlID0gb3V0ZXJtb3N0RW5jbG9zaW5nTGlzdChhZGphY2Vu dFBvcy5kZWVwRXF1aXZhbGVudCgpLm5vZGUoKSk7CisKKyAgICBpZiAoIWxpc3ROb2RlKQorICAg ICAgICByZXR1cm4gMDsKKworICAgIE5vZGUqIHByZXZpb3VzQ2VsbCA9IGVuY2xvc2luZ1RhYmxl Q2VsbChwb3MuZGVlcEVxdWl2YWxlbnQoKSk7CisgICAgTm9kZSogY3VycmVudENlbGwgPSBlbmNs b3NpbmdUYWJsZUNlbGwoYWRqYWNlbnRQb3MuZGVlcEVxdWl2YWxlbnQoKSk7CisKKyAgICBpZiAo IWxpc3ROb2RlLT5oYXNUYWdOYW1lKGxpc3RUYWcpCisgICAgICAgIHx8IGxpc3ROb2RlLT5jb250 YWlucyhwb3MuZGVlcEVxdWl2YWxlbnQoKS5ub2RlKCkpCisgICAgICAgIHx8IHByZXZpb3VzQ2Vs bCAhPSBjdXJyZW50Q2VsbCkKKyAgICAgICAgcmV0dXJuIDA7CisKKyAgICByZXR1cm4gbGlzdE5v ZGU7Cit9CisKIFBhc3NSZWZQdHI8SFRNTEVsZW1lbnQ+IEluc2VydExpc3RDb21tYW5kOjpsaXN0 aWZ5UGFyYWdyYXBoKGNvbnN0IFZpc2libGVQb3NpdGlvbiYgb3JpZ2luYWxTdGFydCwgY29uc3Qg UXVhbGlmaWVkTmFtZSYgbGlzdFRhZykKIHsKICAgICBWaXNpYmxlUG9zaXRpb24gc3RhcnQgPSBz dGFydE9mUGFyYWdyYXBoKG9yaWdpbmFsU3RhcnQpOwogICAgIFZpc2libGVQb3NpdGlvbiBlbmQg PSBlbmRPZlBhcmFncmFwaChzdGFydCk7CiAKICAgICAvLyBDaGVjayBmb3IgYWRqb2luaW5nIGxp c3RzLgotICAgIFZpc2libGVQb3NpdGlvbiBwcmV2aW91c1Bvc2l0aW9uID0gc3RhcnQucHJldmlv dXModHJ1ZSk7Ci0gICAgVmlzaWJsZVBvc2l0aW9uIG5leHRQb3NpdGlvbiA9IGVuZC5uZXh0KHRy dWUpOwogICAgIFJlZlB0cjxIVE1MRWxlbWVudD4gbGlzdEl0ZW1FbGVtZW50ID0gY3JlYXRlTGlz dEl0ZW1FbGVtZW50KGRvY3VtZW50KCkpOwogICAgIFJlZlB0cjxIVE1MRWxlbWVudD4gcGxhY2Vo b2xkZXIgPSBjcmVhdGVCcmVha0VsZW1lbnQoZG9jdW1lbnQoKSk7CiAgICAgYXBwZW5kTm9kZShw bGFjZWhvbGRlciwgbGlzdEl0ZW1FbGVtZW50KTsKLSAgICBFbGVtZW50KiBwcmV2aW91c0xpc3Qg PSBvdXRlcm1vc3RFbmNsb3NpbmdMaXN0KHByZXZpb3VzUG9zaXRpb24uZGVlcEVxdWl2YWxlbnQo KS5ub2RlKCkpOwotICAgIEVsZW1lbnQqIG5leHRMaXN0ID0gb3V0ZXJtb3N0RW5jbG9zaW5nTGlz dChuZXh0UG9zaXRpb24uZGVlcEVxdWl2YWxlbnQoKS5ub2RlKCkpOwotICAgIE5vZGUqIHN0YXJ0 Tm9kZSA9IHN0YXJ0LmRlZXBFcXVpdmFsZW50KCkubm9kZSgpOwotICAgIE5vZGUqIHByZXZpb3Vz Q2VsbCA9IGVuY2xvc2luZ1RhYmxlQ2VsbChwcmV2aW91c1Bvc2l0aW9uLmRlZXBFcXVpdmFsZW50 KCkpOwotICAgIE5vZGUqIG5leHRDZWxsID0gZW5jbG9zaW5nVGFibGVDZWxsKG5leHRQb3NpdGlv bi5kZWVwRXF1aXZhbGVudCgpKTsKLSAgICBOb2RlKiBjdXJyZW50Q2VsbCA9IGVuY2xvc2luZ1Rh YmxlQ2VsbChzdGFydC5kZWVwRXF1aXZhbGVudCgpKTsKLSAgICBpZiAocHJldmlvdXNMaXN0ICYm ICghcHJldmlvdXNMaXN0LT5oYXNUYWdOYW1lKGxpc3RUYWcpIHx8IHN0YXJ0Tm9kZS0+aXNEZXNj ZW5kYW50T2YocHJldmlvdXNMaXN0KSB8fCBwcmV2aW91c0NlbGwgIT0gY3VycmVudENlbGwpKQot ICAgICAgICBwcmV2aW91c0xpc3QgPSAwOwotICAgIGlmIChuZXh0TGlzdCAmJiAoIW5leHRMaXN0 LT5oYXNUYWdOYW1lKGxpc3RUYWcpIHx8IHN0YXJ0Tm9kZS0+aXNEZXNjZW5kYW50T2YobmV4dExp c3QpIHx8IG5leHRDZWxsICE9IGN1cnJlbnRDZWxsKSkKLSAgICAgICAgbmV4dExpc3QgPSAwOwor CiAgICAgLy8gUGxhY2UgbGlzdCBpdGVtIGludG8gYWRqb2luaW5nIGxpc3RzLgorICAgIEVsZW1l bnQqIHByZXZpb3VzTGlzdCA9IGFkamFjZW50RW5jbG9zaW5nTGlzdChzdGFydC5kZWVwRXF1aXZh bGVudCgpLCBzdGFydC5wcmV2aW91cyh0cnVlKSwgbGlzdFRhZyk7CisgICAgRWxlbWVudCogbmV4 dExpc3QgPSBhZGphY2VudEVuY2xvc2luZ0xpc3Qoc3RhcnQuZGVlcEVxdWl2YWxlbnQoKSwgZW5k Lm5leHQodHJ1ZSksIGxpc3RUYWcpOwogICAgIFJlZlB0cjxIVE1MRWxlbWVudD4gbGlzdEVsZW1l bnQ7CiAgICAgaWYgKHByZXZpb3VzTGlzdCkKICAgICAgICAgYXBwZW5kTm9kZShsaXN0SXRlbUVs ZW1lbnQsIHByZXZpb3VzTGlzdCk7CkBAIC0yNjgsOCArMjgxLDYgQEAgUGFzc1JlZlB0cjxIVE1M RWxlbWVudD4gSW5zZXJ0TGlzdENvbW1hbgogICAgICAgICAvLyBVcGRhdGUgdGhlIHN0YXJ0IG9m IGNvbnRlbnQsIHNvIHdlIGRvbid0IHRyeSB0byBtb3ZlIHRoZSBsaXN0IGludG8gaXRzZWxmLiAg YnVnIDE5MDY2CiAgICAgICAgIGlmIChpbnNlcnRpb25Qb3MgPT0gc3RhcnQuZGVlcEVxdWl2YWxl bnQoKSkKICAgICAgICAgICAgIHN0YXJ0ID0gc3RhcnRPZlBhcmFncmFwaChvcmlnaW5hbFN0YXJ0 KTsKLSAgICAgICAgcHJldmlvdXNMaXN0ID0gb3V0ZXJtb3N0RW5jbG9zaW5nTGlzdChwcmV2aW91 c1Bvc2l0aW9uLmRlZXBFcXVpdmFsZW50KCkubm9kZSgpLCBlbmNsb3NpbmdMaXN0KGxpc3RFbGVt ZW50LmdldCgpKSk7Ci0gICAgICAgIG5leHRMaXN0ID0gb3V0ZXJtb3N0RW5jbG9zaW5nTGlzdChu ZXh0UG9zaXRpb24uZGVlcEVxdWl2YWxlbnQoKS5ub2RlKCksIGVuY2xvc2luZ0xpc3QobGlzdEVs ZW1lbnQuZ2V0KCkpKTsKICAgICB9CiAKICAgICBtb3ZlUGFyYWdyYXBoKHN0YXJ0LCBlbmQsIFZp c2libGVQb3NpdGlvbihQb3NpdGlvbihwbGFjZWhvbGRlci5nZXQoKSwgMCkpLCB0cnVlKTsKQEAg LTI4MSw2ICsyOTIsOCBAQCBQYXNzUmVmUHRyPEhUTUxFbGVtZW50PiBJbnNlcnRMaXN0Q29tbWFu CiAgICAgICAgIGxpc3RFbGVtZW50ID0gbV9saXN0RWxlbWVudDsKIAogICAgIGlmIChsaXN0RWxl bWVudCkgeworICAgICAgICBwcmV2aW91c0xpc3QgPSBsaXN0RWxlbWVudC0+cHJldmlvdXNFbGVt ZW50U2libGluZygpOworICAgICAgICBuZXh0TGlzdCA9IGxpc3RFbGVtZW50LT5uZXh0RWxlbWVu dFNpYmxpbmcoKTsKICAgICAgICAgaWYgKGNhbk1lcmdlTGlzdHMocHJldmlvdXNMaXN0LCBsaXN0 RWxlbWVudC5nZXQoKSkpCiAgICAgICAgICAgICBtZXJnZUlkZW50aWNhbEVsZW1lbnRzKHByZXZp b3VzTGlzdCwgbGlzdEVsZW1lbnQuZ2V0KCkpOwogICAgICAgICBpZiAoY2FuTWVyZ2VMaXN0cyhs aXN0RWxlbWVudC5nZXQoKSwgbmV4dExpc3QpKQo=