36948 2010-04-01 04:47:12 -0700 Refactoring: Position::primaryDirection() should be extracted. 2010-04-01 22:30:35 -0700 1 1 1 Unclassified WebKit DOM 528+ (Nightly build) PC All RESOLVED FIXED P3 Normal --- 0 morrita webkit-unassigned ap commit-queue hamaji mitz oldest_to_newest 207083 0 morrita 2010-04-01 04:47:12 -0700 TextDirection typed variables named "primaryDirection" are computed several part on our code-base. It should be extract to a function. 207086 1 52285 morrita 2010-04-01 04:52:05 -0700 Created attachment 52285 patch v0 207149 2 52285 hamaji 2010-04-01 07:50:01 -0700 Comment on attachment 52285 patch v0 Nice! 207163 3 52285 hamaji 2010-04-01 08:14:55 -0700 Comment on attachment 52285 patch v0 Clearing flags on attachment: 52285 Committed r56915: <http://trac.webkit.org/changeset/56915> 207165 4 hamaji 2010-04-01 08:15:05 -0700 All reviewed patches have been landed. Closing bug. 207168 5 52285 commit-queue 2010-04-01 08:23:00 -0700 Comment on attachment 52285 patch v0 Rejecting patch 52285 from commit-queue. Unexpected failure when landing patch! Please file a bug against webkit-patch. Failed to run "['WebKitTools/Scripts/webkit-patch', '--status-host=webkit-commit-queue.appspot.com', 'land-attachment', '--force-clean', '--non-interactive', '--build-style=both', '--quiet', '52285', '--parent-command=commit-queue', '--no-update']" exit_code: 1 Last 500 characters of output: cgi?id=52285&action=edit Fetching: https://bugs.webkit.org/show_bug.cgi?id=36948&ctype=xml Processing 1 patch from 1 bug. Cleaning working directory Processing patch 52285 from bug 36948. NOBODY (OOPS!) found in /Users/eseidel/Projects/CommitQueue/WebCore/ChangeLog does not appear to be a valid reviewer according to committers.py. ERROR: /Users/eseidel/Projects/CommitQueue/WebCore/ChangeLog neither lists a valid reviewer nor contains the string "Unreviewed" or "Rubber stamp" (case insensitive). 207234 6 ap 2010-04-01 10:28:48 -0700 It's definitely a great change to extract this code into a function. But I'm not sure if Position is a good place for it. Of all data members in Position, this code only uses m_node, so I wonder if direction is conceptually a function of position. It's also easy to confuse with VisiblePosition affinity. On the other hand, it's only used in Position and VisiblePosition code. Hmm. + for (const RenderObject* r = node()->renderer(); r; r = r->parent()) { We don't usually use accessor functions in class implementation, should preferably be m_anchorNode->renderer(). 207570 7 52285 eric 2010-04-01 18:19:25 -0700 Comment on attachment 52285 patch v0 Looks like cq+ was just set prematurely. 207675 8 morrita 2010-04-01 22:30:35 -0700 Hi eric, > Looks like cq+ was just set prematurely. Umm. I filed the fix on Bug 37011 and submitted a patch to do that small cleanup, just for taking advantage of commit-queue. 52285 2010-04-01 04:52:05 -0700 2010-04-01 18:19:25 -0700 patch v0 bug-36948-20100401205203.patch text/plain 3984 morrita ZGlmZiAtLWdpdCBhL1dlYkNvcmUvQ2hhbmdlTG9nIGIvV2ViQ29yZS9DaGFuZ2VMb2cKaW5kZXgg MDkxN2QwNS4uNmQ0NTIzNyAxMDA2NDQKLS0tIGEvV2ViQ29yZS9DaGFuZ2VMb2cKKysrIGIvV2Vi Q29yZS9DaGFuZ2VMb2cKQEAgLTEsMyArMSwyMCBAQAorMjAxMC0wNC0wMSAgTU9SSVRBIEhhamlt ZSAgPG1vcnJpdGFAZ29vZ2xlLmNvbT4KKworICAgICAgICBSZXZpZXdlZCBieSBOT0JPRFkgKE9P UFMhKS4KKworICAgICAgICBodHRwczovL2J1Z3Mud2Via2l0Lm9yZy9zaG93X2J1Zy5jZ2k/aWQ9 MzY5NDgKKyAgICAgICAgUmVmYWN0b3Jpbmc6IFBvc2l0aW9uOjpwcmltYXJ5RGlyZWN0aW9uKCkg c2hvdWxkIGJlIGV4dHJhY3RlZC4KKworICAgICAgICBObyBuZXcgdGVzdHMuIE5vIGZ1bmN0aW5h bCBjaGFuZ2UuCisKKyAgICAgICAgKiBkb20vUG9zaXRpb24uY3BwOgorICAgICAgICAoV2ViQ29y ZTo6UG9zaXRpb246OmdldElubGluZUJveEFuZE9mZnNldCk6CisgICAgICAgIChXZWJDb3JlOjpQ b3NpdGlvbjo6cHJpbWFyeURpcmVjdGlvbik6IEFkZGVkCisgICAgICAgICogZG9tL1Bvc2l0aW9u Lmg6CisgICAgICAgICogZWRpdGluZy9WaXNpYmxlUG9zaXRpb24uY3BwOgorICAgICAgICAoV2Vi Q29yZTo6VmlzaWJsZVBvc2l0aW9uOjpsZWZ0VmlzdWFsbHlEaXN0aW5jdENhbmRpZGF0ZSk6Cisg ICAgICAgIChXZWJDb3JlOjpWaXNpYmxlUG9zaXRpb246OnJpZ2h0VmlzdWFsbHlEaXN0aW5jdENh bmRpZGF0ZSk6CisKIDIwMTAtMDMtMzEgIE5pa29sYXMgWmltbWVybWFubiAgPG56aW1tZXJtYW5u QHJpbS5jb20+CiAKICAgICAgICAgUmV2aWV3ZWQgYnkgRGlyayBTY2h1bHplLgpkaWZmIC0tZ2l0 IGEvV2ViQ29yZS9kb20vUG9zaXRpb24uY3BwIGIvV2ViQ29yZS9kb20vUG9zaXRpb24uY3BwCmlu ZGV4IDQ2N2MwNmQuLjNmMGEwZTggMTAwNjQ0Ci0tLSBhL1dlYkNvcmUvZG9tL1Bvc2l0aW9uLmNw cAorKysgYi9XZWJDb3JlL2RvbS9Qb3NpdGlvbi5jcHAKQEAgLTk0NiwxNCArOTQ2LDcgQEAgUG9z aXRpb24gUG9zaXRpb246OnRyYWlsaW5nV2hpdGVzcGFjZVBvc2l0aW9uKEVBZmZpbml0eSwgYm9v bCBjb25zaWRlck5vbkNvbGxhcHMKIAogdm9pZCBQb3NpdGlvbjo6Z2V0SW5saW5lQm94QW5kT2Zm c2V0KEVBZmZpbml0eSBhZmZpbml0eSwgSW5saW5lQm94KiYgaW5saW5lQm94LCBpbnQmIGNhcmV0 T2Zmc2V0KSBjb25zdAogewotICAgIFRleHREaXJlY3Rpb24gcHJpbWFyeURpcmVjdGlvbiA9IExU UjsKLSAgICBmb3IgKFJlbmRlck9iamVjdCogciA9IG5vZGUoKS0+cmVuZGVyZXIoKTsgcjsgciA9 IHItPnBhcmVudCgpKSB7Ci0gICAgICAgIGlmIChyLT5pc0Jsb2NrRmxvdygpKSB7Ci0gICAgICAg ICAgICBwcmltYXJ5RGlyZWN0aW9uID0gci0+c3R5bGUoKS0+ZGlyZWN0aW9uKCk7Ci0gICAgICAg ICAgICBicmVhazsKLSAgICAgICAgfQotICAgIH0KLSAgICBnZXRJbmxpbmVCb3hBbmRPZmZzZXQo YWZmaW5pdHksIHByaW1hcnlEaXJlY3Rpb24sIGlubGluZUJveCwgY2FyZXRPZmZzZXQpOworICAg IGdldElubGluZUJveEFuZE9mZnNldChhZmZpbml0eSwgcHJpbWFyeURpcmVjdGlvbigpLCBpbmxp bmVCb3gsIGNhcmV0T2Zmc2V0KTsKIH0KIAogc3RhdGljIGJvb2wgaXNOb25UZXh0TGVhZkNoaWxk KFJlbmRlck9iamVjdCogb2JqZWN0KQpAQCAtMTE0Nyw2ICsxMTQwLDIwIEBAIHZvaWQgUG9zaXRp b246OmdldElubGluZUJveEFuZE9mZnNldChFQWZmaW5pdHkgYWZmaW5pdHksIFRleHREaXJlY3Rp b24gcHJpbWFyeURpCiAgICAgfQogfQogCitUZXh0RGlyZWN0aW9uIFBvc2l0aW9uOjpwcmltYXJ5 RGlyZWN0aW9uKCkgY29uc3QKK3sKKyAgICBUZXh0RGlyZWN0aW9uIHByaW1hcnlEaXJlY3Rpb24g PSBMVFI7CisgICAgZm9yIChjb25zdCBSZW5kZXJPYmplY3QqIHIgPSBub2RlKCktPnJlbmRlcmVy KCk7IHI7IHIgPSByLT5wYXJlbnQoKSkgeworICAgICAgICBpZiAoci0+aXNCbG9ja0Zsb3coKSkg eworICAgICAgICAgICAgcHJpbWFyeURpcmVjdGlvbiA9IHItPnN0eWxlKCktPmRpcmVjdGlvbigp OworICAgICAgICAgICAgYnJlYWs7CisgICAgICAgIH0KKyAgICB9CisKKyAgICByZXR1cm4gcHJp bWFyeURpcmVjdGlvbjsKK30KKworCiB2b2lkIFBvc2l0aW9uOjpkZWJ1Z1Bvc2l0aW9uKGNvbnN0 IGNoYXIqIG1zZykgY29uc3QKIHsKICAgICBpZiAoaXNOdWxsKCkpCmRpZmYgLS1naXQgYS9XZWJD b3JlL2RvbS9Qb3NpdGlvbi5oIGIvV2ViQ29yZS9kb20vUG9zaXRpb24uaAppbmRleCBmYzU4NDlm Li41OTFiMGU2IDEwMDY0NAotLS0gYS9XZWJDb3JlL2RvbS9Qb3NpdGlvbi5oCisrKyBiL1dlYkNv cmUvZG9tL1Bvc2l0aW9uLmgKQEAgLTE1OCw2ICsxNTgsOCBAQCBwdWJsaWM6CiAgICAgdm9pZCBn ZXRJbmxpbmVCb3hBbmRPZmZzZXQoRUFmZmluaXR5LCBJbmxpbmVCb3gqJiwgaW50JiBjYXJldE9m ZnNldCkgY29uc3Q7CiAgICAgdm9pZCBnZXRJbmxpbmVCb3hBbmRPZmZzZXQoRUFmZmluaXR5LCBU ZXh0RGlyZWN0aW9uIHByaW1hcnlEaXJlY3Rpb24sIElubGluZUJveComLCBpbnQmIGNhcmV0T2Zm c2V0KSBjb25zdDsKIAorICAgIFRleHREaXJlY3Rpb24gcHJpbWFyeURpcmVjdGlvbigpIGNvbnN0 OworCiAgICAgc3RhdGljIGJvb2wgaGFzUmVuZGVyZWROb25Bbm9ueW1vdXNEZXNjZW5kYW50c1dp dGhIZWlnaHQoUmVuZGVyT2JqZWN0Kik7CiAgICAgc3RhdGljIGJvb2wgbm9kZUlzVXNlclNlbGVj dE5vbmUoTm9kZSopOwogICAgIApkaWZmIC0tZ2l0IGEvV2ViQ29yZS9lZGl0aW5nL1Zpc2libGVQ b3NpdGlvbi5jcHAgYi9XZWJDb3JlL2VkaXRpbmcvVmlzaWJsZVBvc2l0aW9uLmNwcAppbmRleCA1 MWFiNjRiLi4yNTI5OThmIDEwMDY0NAotLS0gYS9XZWJDb3JlL2VkaXRpbmcvVmlzaWJsZVBvc2l0 aW9uLmNwcAorKysgYi9XZWJDb3JlL2VkaXRpbmcvVmlzaWJsZVBvc2l0aW9uLmNwcApAQCAtMTEw LDEzICsxMTAsNyBAQCBQb3NpdGlvbiBWaXNpYmxlUG9zaXRpb246OmxlZnRWaXN1YWxseURpc3Rp bmN0Q2FuZGlkYXRlKCkgY29uc3QKICAgICAgICAgcmV0dXJuIFBvc2l0aW9uKCk7CiAKICAgICBQ b3NpdGlvbiBkb3duc3RyZWFtU3RhcnQgPSBwLmRvd25zdHJlYW0oKTsKLSAgICBUZXh0RGlyZWN0 aW9uIHByaW1hcnlEaXJlY3Rpb24gPSBMVFI7Ci0gICAgZm9yIChSZW5kZXJPYmplY3QqIHIgPSBw Lm5vZGUoKS0+cmVuZGVyZXIoKTsgcjsgciA9IHItPnBhcmVudCgpKSB7Ci0gICAgICAgIGlmIChy LT5pc0Jsb2NrRmxvdygpKSB7Ci0gICAgICAgICAgICBwcmltYXJ5RGlyZWN0aW9uID0gci0+c3R5 bGUoKS0+ZGlyZWN0aW9uKCk7Ci0gICAgICAgICAgICBicmVhazsKLSAgICAgICAgfQotICAgIH0K KyAgICBUZXh0RGlyZWN0aW9uIHByaW1hcnlEaXJlY3Rpb24gPSBwLnByaW1hcnlEaXJlY3Rpb24o KTsKIAogICAgIHdoaWxlICh0cnVlKSB7CiAgICAgICAgIElubGluZUJveCogYm94OwpAQCAtMjUy LDEzICsyNDYsNyBAQCBQb3NpdGlvbiBWaXNpYmxlUG9zaXRpb246OnJpZ2h0VmlzdWFsbHlEaXN0 aW5jdENhbmRpZGF0ZSgpIGNvbnN0CiAgICAgICAgIHJldHVybiBQb3NpdGlvbigpOwogCiAgICAg UG9zaXRpb24gZG93bnN0cmVhbVN0YXJ0ID0gcC5kb3duc3RyZWFtKCk7Ci0gICAgVGV4dERpcmVj dGlvbiBwcmltYXJ5RGlyZWN0aW9uID0gTFRSOwotICAgIGZvciAoUmVuZGVyT2JqZWN0KiByID0g cC5ub2RlKCktPnJlbmRlcmVyKCk7IHI7IHIgPSByLT5wYXJlbnQoKSkgewotICAgICAgICBpZiAo ci0+aXNCbG9ja0Zsb3coKSkgewotICAgICAgICAgICAgcHJpbWFyeURpcmVjdGlvbiA9IHItPnN0 eWxlKCktPmRpcmVjdGlvbigpOwotICAgICAgICAgICAgYnJlYWs7Ci0gICAgICAgIH0KLSAgICB9 CisgICAgVGV4dERpcmVjdGlvbiBwcmltYXJ5RGlyZWN0aW9uID0gcC5wcmltYXJ5RGlyZWN0aW9u KCk7CiAKICAgICB3aGlsZSAodHJ1ZSkgewogICAgICAgICBJbmxpbmVCb3gqIGJveDsK