66096 2011-08-11 14:01:27 -0700 Add a way to extend DOM objects in garden-o-matic. 2011-08-12 10:34:23 -0700 1 1 1 Unclassified WebKit Tools / Tests 528+ (Nightly build) Unspecified Unspecified RESOLVED FIXED P2 Normal --- 64188 66144 1 dglazkov dglazkov abarth arv oldest_to_newest 450021 0 dglazkov 2011-08-11 14:01:27 -0700 Add a way to extend DOM objects in garden-o-matic. 450023 1 103668 dglazkov 2011-08-11 14:02:27 -0700 Created attachment 103668 Patch 450095 2 103668 sam 2011-08-11 15:35:46 -0700 Comment on attachment 103668 Patch View in context: https://bugs.webkit.org/attachment.cgi?id=103668&action=review > Tools/BuildSlaveSupport/build.webkit.org-config/public_html/TestFailures/base.js:222 > +// arv r0x0r 4eva. I am not sure future readers of this code will understand this comment. > Tools/ChangeLog:6 > + This is loosely based on http://ajaxian.com/archives/web-ninja-interview-erik-arvidsson. Did you copy it? Does it have a license? 450100 3 103668 abarth 2011-08-11 15:42:40 -0700 Comment on attachment 103668 Patch View in context: https://bugs.webkit.org/attachment.cgi?id=103668&action=review > Tools/BuildSlaveSupport/build.webkit.org-config/public_html/TestFailures/base.js:232 > + function f() { > + var el = typeof base == 'string' ? document.createElement(base) : base.call(this); > + f.prototype.__proto__ = el.__proto__; > + el.__proto__ = f.prototype; > + el.init && el.init(); > + return el; > + } f and el are pretty lame variable names. Also, this construction leaks the implementation detail "f" in the API. > Tools/BuildSlaveSupport/build.webkit.org-config/public_html/TestFailures/base_unittests.js:223 > + init: function() > + { We've been putting the { on the same line in these sorts of declarations. > Tools/BuildSlaveSupport/build.webkit.org-config/public_html/TestFailures/base_unittests.js:226 > + method: function(msg) msg => message ? > Tools/BuildSlaveSupport/build.webkit.org-config/public_html/TestFailures/base_unittests.js:241 > + ok(document.body.lastChild.innerHTML == "awesome"); > + ok(document.body.lastChild.method() == 42); These should use equals > Tools/BuildSlaveSupport/build.webkit.org-config/public_html/TestFailures/base_unittests.js:247 > + ok(document.body.lastChild.innerHTML == "awesome"); > + ok(document.body.lastChild.method() == 42); > + ok(document.body.lastChild.className == "like"); Same here. 450102 4 103668 abarth 2011-08-11 15:43:35 -0700 Comment on attachment 103668 Patch This looks fine, assuming ther eare no licensing problems and you address Sam and my comments above. 450265 5 dglazkov 2011-08-11 20:56:48 -0700 (In reply to comment #2) > (From update of attachment 103668 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=103668&action=review > > > Tools/BuildSlaveSupport/build.webkit.org-config/public_html/TestFailures/base.js:222 > > +// arv r0x0r 4eva. > > I am not sure future readers of this code will understand this comment. I'll make it more readable :) > > > Tools/ChangeLog:6 > > + This is loosely based on http://ajaxian.com/archives/web-ninja-interview-erik-arvidsson. > > Did you copy it? Does it have a license? I made wild extrapolations of the initial idea. I hope Arv blesses this himself. 450267 6 103668 dglazkov 2011-08-11 21:01:25 -0700 Comment on attachment 103668 Patch View in context: https://bugs.webkit.org/attachment.cgi?id=103668&action=review >> Tools/BuildSlaveSupport/build.webkit.org-config/public_html/TestFailures/base.js:232 >> + } > > f and el are pretty lame variable names. Also, this construction leaks the implementation detail "f" in the API. I'll rename. Though I don't see a way of not leaking the f. That's the whole point of the trick. >> Tools/BuildSlaveSupport/build.webkit.org-config/public_html/TestFailures/base_unittests.js:223 >> + { > > We've been putting the { on the same line in these sorts of declarations. Really? That seems to be different from the WebKit style. I'll change. 450277 7 103668 abarth 2011-08-11 21:22:17 -0700 Comment on attachment 103668 Patch View in context: https://bugs.webkit.org/attachment.cgi?id=103668&action=review >>> Tools/BuildSlaveSupport/build.webkit.org-config/public_html/TestFailures/base.js:232 >>> + } >> >> f and el are pretty lame variable names. Also, this construction leaks the implementation detail "f" in the API. > > I'll rename. Though I don't see a way of not leaking the f. That's the whole point of the trick. Just change: function f() { to var f = function () { Just like magic, we no longer leak the name "f". 450489 8 arv 2011-08-12 10:27:11 -0700 (In reply to comment #2) > (From update of attachment 103668 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=103668&action=review > > > Tools/BuildSlaveSupport/build.webkit.org-config/public_html/TestFailures/base.js:222 > > +// arv r0x0r 4eva. > > I am not sure future readers of this code will understand this comment. > > > Tools/ChangeLog:6 > > + This is loosely based on http://ajaxian.com/archives/web-ninja-interview-erik-arvidsson. > > Did you copy it? Does it have a license? The original code was written for Chromium so it has a BSD-style license: http://www.google.com/codesearch#OAMlx_jo-ck/src/chrome/browser/resources/shared/js/cr/ui.js&type=cs 450493 9 dglazkov 2011-08-12 10:30:37 -0700 (In reply to comment #8) > (In reply to comment #2) > > (From update of attachment 103668 [details] [details]) > > View in context: https://bugs.webkit.org/attachment.cgi?id=103668&action=review > > > > > Tools/BuildSlaveSupport/build.webkit.org-config/public_html/TestFailures/base.js:222 > > > +// arv r0x0r 4eva. > > > > I am not sure future readers of this code will understand this comment. > > > > > Tools/ChangeLog:6 > > > + This is loosely based on http://ajaxian.com/archives/web-ninja-interview-erik-arvidsson. > > > > Did you copy it? Does it have a license? > > The original code was written for Chromium so it has a BSD-style license: > > http://www.google.com/codesearch#OAMlx_jo-ck/src/chrome/browser/resources/shared/js/cr/ui.js&type=cs I'll update the link in ChangeLog and mention this in the comment. 450495 10 dglazkov 2011-08-12 10:34:23 -0700 Committed r92972: <http://trac.webkit.org/changeset/92972> 103668 2011-08-11 14:02:27 -0700 2011-08-11 21:22:17 -0700 Patch bug-66096-20110811140226.patch text/plain 3370 dglazkov U3VidmVyc2lvbiBSZXZpc2lvbjogOTI4NjEKZGlmZiAtLWdpdCBhL1Rvb2xzL0J1aWxkU2xhdmVT dXBwb3J0L2J1aWxkLndlYmtpdC5vcmctY29uZmlnL3B1YmxpY19odG1sL1Rlc3RGYWlsdXJlcy9i YXNlLmpzIGIvVG9vbHMvQnVpbGRTbGF2ZVN1cHBvcnQvYnVpbGQud2Via2l0Lm9yZy1jb25maWcv cHVibGljX2h0bWwvVGVzdEZhaWx1cmVzL2Jhc2UuanMKaW5kZXggNDJlNGNiMGViZDhjNTY3ZTBj YjM3ZDg5MmUwZmNjMDNjZDMwZjMwZC4uOGNlNjllNjk0Yzc4N2YzMjU3Y2QxMjYyMjRlZmIzYjJj YjNlYzBiNSAxMDA2NDQKLS0tIGEvVG9vbHMvQnVpbGRTbGF2ZVN1cHBvcnQvYnVpbGQud2Via2l0 Lm9yZy1jb25maWcvcHVibGljX2h0bWwvVGVzdEZhaWx1cmVzL2Jhc2UuanMKKysrIGIvVG9vbHMv QnVpbGRTbGF2ZVN1cHBvcnQvYnVpbGQud2Via2l0Lm9yZy1jb25maWcvcHVibGljX2h0bWwvVGVz dEZhaWx1cmVzL2Jhc2UuanMKQEAgLTIxOSw0ICsyMTksMjAgQEAgYmFzZS5Bc3luY2hyb25vdXND YWNoZS5wcm90b3R5cGUuZ2V0ID0gZnVuY3Rpb24oa2V5LCBjYWxsYmFjaykKICAgICB9KTsKIH07 CiAKKy8vIGFydiByMHgwciA0ZXZhLgorLy8gRklYTUU6IFVzZSBwcm9wZXIgY29uc3RydWN0b3Ig c2VtYW50aWNzLgorYmFzZS5leHRlbmRzID0gZnVuY3Rpb24oYmFzZSwgcHJvdG90eXBlKQorewor ICAgIGZ1bmN0aW9uIGYoKSB7CisgICAgICAgIHZhciBlbCA9IHR5cGVvZiBiYXNlID09ICdzdHJp bmcnID8gZG9jdW1lbnQuY3JlYXRlRWxlbWVudChiYXNlKSA6IGJhc2UuY2FsbCh0aGlzKTsKKyAg ICAgICAgZi5wcm90b3R5cGUuX19wcm90b19fID0gZWwuX19wcm90b19fOworICAgICAgICBlbC5f X3Byb3RvX18gPSBmLnByb3RvdHlwZTsKKyAgICAgICAgZWwuaW5pdCAmJiBlbC5pbml0KCk7Cisg ICAgICAgIHJldHVybiBlbDsKKyAgICB9CisKKyAgICBmLnByb3RvdHlwZSA9IHByb3RvdHlwZTsK KyAgICByZXR1cm4gZjsKK30KKwogfSkoKTsKZGlmZiAtLWdpdCBhL1Rvb2xzL0J1aWxkU2xhdmVT dXBwb3J0L2J1aWxkLndlYmtpdC5vcmctY29uZmlnL3B1YmxpY19odG1sL1Rlc3RGYWlsdXJlcy9i YXNlX3VuaXR0ZXN0cy5qcyBiL1Rvb2xzL0J1aWxkU2xhdmVTdXBwb3J0L2J1aWxkLndlYmtpdC5v cmctY29uZmlnL3B1YmxpY19odG1sL1Rlc3RGYWlsdXJlcy9iYXNlX3VuaXR0ZXN0cy5qcwppbmRl eCAxODgzZWM0ZGQyOTdlOWM0YjAzOGM5NmUzMTYxYjFmYTJmY2Y2ZDI0Li42ZDA4ZjBlN2YyMzdj NDBlOTk2ZGM3ZDVjOTQ1NjAwZWM4ZGZhMDYyIDEwMDY0NAotLS0gYS9Ub29scy9CdWlsZFNsYXZl U3VwcG9ydC9idWlsZC53ZWJraXQub3JnLWNvbmZpZy9wdWJsaWNfaHRtbC9UZXN0RmFpbHVyZXMv YmFzZV91bml0dGVzdHMuanMKKysrIGIvVG9vbHMvQnVpbGRTbGF2ZVN1cHBvcnQvYnVpbGQud2Vi a2l0Lm9yZy1jb25maWcvcHVibGljX2h0bWwvVGVzdEZhaWx1cmVzL2Jhc2VfdW5pdHRlc3RzLmpz CkBAIC0yMTYsNCArMjE2LDM2IEBAIHRlc3QoImZpbHRlclRyZWUiLCAyLCBmdW5jdGlvbigpIHsK ICAgICB9KTsKIH0pOwogCit0ZXN0KCJleHRlbmRzIiwgNSwgZnVuY3Rpb24oKSB7CisKKyAgICB2 YXIgTGlrZURpdiA9IGJhc2UuZXh0ZW5kcygiZGl2IiwgeworICAgICAgICBpbml0OiBmdW5jdGlv bigpCisgICAgICAgIHsKKyAgICAgICAgICAgIHRoaXMudGV4dENvbnRlbnQgPSAiYXdlc29tZSI7 CisgICAgICAgIH0sCisgICAgICAgIG1ldGhvZDogZnVuY3Rpb24obXNnKQorICAgICAgICB7Cisg ICAgICAgICAgICByZXR1cm4gNDI7CisgICAgICAgIH0KKyAgICB9KTsKKworICAgIHZhciBMaWtl TGlrZURpdiA9IGJhc2UuZXh0ZW5kcyhMaWtlRGl2LCB7CisgICAgICAgIGluaXQ6IGZ1bmN0aW9u KCkKKyAgICAgICAgeworICAgICAgICAgICAgdGhpcy5jbGFzc05hbWUgPSAibGlrZSI7CisgICAg ICAgIH0KKyAgICB9KTsKKworICAgIGRvY3VtZW50LmJvZHkuYXBwZW5kQ2hpbGQobmV3IExpa2VE aXYoKSk7CisgICAgb2soZG9jdW1lbnQuYm9keS5sYXN0Q2hpbGQuaW5uZXJIVE1MID09ICJhd2Vz b21lIik7CisgICAgb2soZG9jdW1lbnQuYm9keS5sYXN0Q2hpbGQubWV0aG9kKCkgPT0gNDIpOwor ICAgIGRvY3VtZW50LmJvZHkucmVtb3ZlQ2hpbGQoZG9jdW1lbnQuYm9keS5sYXN0Q2hpbGQpOwor CisgICAgZG9jdW1lbnQuYm9keS5hcHBlbmRDaGlsZChuZXcgTGlrZUxpa2VEaXYoKSk7CisgICAg b2soZG9jdW1lbnQuYm9keS5sYXN0Q2hpbGQuaW5uZXJIVE1MID09ICJhd2Vzb21lIik7CisgICAg b2soZG9jdW1lbnQuYm9keS5sYXN0Q2hpbGQubWV0aG9kKCkgPT0gNDIpOworICAgIG9rKGRvY3Vt ZW50LmJvZHkubGFzdENoaWxkLmNsYXNzTmFtZSA9PSAibGlrZSIpOworICAgIGRvY3VtZW50LmJv ZHkucmVtb3ZlQ2hpbGQoZG9jdW1lbnQuYm9keS5sYXN0Q2hpbGQpOworfSk7CisKIH0pKCk7CmRp ZmYgLS1naXQgYS9Ub29scy9DaGFuZ2VMb2cgYi9Ub29scy9DaGFuZ2VMb2cKaW5kZXggOGMzOWRm OGM0NGI2ZTg4ZmVjMTZmYjE0Nzc3YTk3NWE3NGI3OGMwYS4uZTM0YmRiZjc1NjI5NmI5MTAzZjVh MTQ0NTk5MDhhMjAwOTI5YWQ5OCAxMDA2NDQKLS0tIGEvVG9vbHMvQ2hhbmdlTG9nCisrKyBiL1Rv b2xzL0NoYW5nZUxvZwpAQCAtMSw1ICsxLDE3IEBACiAyMDExLTA4LTExICBEaW1pdHJpIEdsYXpr b3YgIDxkZ2xhemtvdkBjaHJvbWl1bS5vcmc+CiAKKyAgICAgICAgQWRkIGEgd2F5IHRvIGV4dGVu ZCBET00gb2JqZWN0cyBpbiBnYXJkZW4tby1tYXRpYy4KKyAgICAgICAgaHR0cHM6Ly9idWdzLndl YmtpdC5vcmcvc2hvd19idWcuY2dpP2lkPTY2MDk2CisKKyAgICAgICAgVGhpcyBpcyBsb29zZWx5 IGJhc2VkIG9uIGh0dHA6Ly9hamF4aWFuLmNvbS9hcmNoaXZlcy93ZWItbmluamEtaW50ZXJ2aWV3 LWVyaWstYXJ2aWRzc29uLgorCisgICAgICAgIFJldmlld2VkIGJ5IE5PQk9EWSAoT09QUyEpLgor CisgICAgICAgICogQnVpbGRTbGF2ZVN1cHBvcnQvYnVpbGQud2Via2l0Lm9yZy1jb25maWcvcHVi bGljX2h0bWwvVGVzdEZhaWx1cmVzL2Jhc2UuanM6IEFkZGVkIGJhc2UuZXh0ZW5kcy4KKyAgICAg ICAgKiBCdWlsZFNsYXZlU3VwcG9ydC9idWlsZC53ZWJraXQub3JnLWNvbmZpZy9wdWJsaWNfaHRt bC9UZXN0RmFpbHVyZXMvYmFzZV91bml0dGVzdHMuanM6IEFkZGVkIHRlc3RzLgorCisyMDExLTA4 LTExICBEaW1pdHJpIEdsYXprb3YgIDxkZ2xhemtvdkBjaHJvbWl1bS5vcmc+CisKICAgICAgICAg TmV3IGV4cGVjdGF0aW9uIGVudHJpZXMgc2hvdWxkIGJlIHBsYWNlZCBuZXh0IHRvIGV4aXN0aW5n IG9uZXMuCiAgICAgICAgIGh0dHBzOi8vYnVncy53ZWJraXQub3JnL3Nob3dfYnVnLmNnaT9pZD02 NjA2NgogCg==