51142 2010-12-15 15:17:00 -0800 size status bubble to it's contents on the code review page 2010-12-17 10:41:48 -0800 1 1 1 Unclassified WebKit New Bugs 528+ (Nightly build) Other OS X 10.5 RESOLVED FIXED P2 Normal --- 1 ojan ojan ossy tonikitoo oldest_to_newest 322846 0 ojan 2010-12-15 15:17:00 -0800 size status bubble to it's contents on the code review page 322849 1 76692 ojan 2010-12-15 15:17:32 -0800 Created attachment 76692 Patch 322850 2 76693 ojan 2010-12-15 15:18:25 -0800 Created attachment 76693 Patch 322860 3 76693 abarth 2010-12-15 15:23:38 -0800 Comment on attachment 76693 Patch View in context: https://bugs.webkit.org/attachment.cgi?id=76693&action=review > BugsSite/code-review.js:259 > + $('.statusBubble')[0].style.height = e.data.height; > + $('.statusBubble')[0].style.width = e.data.width; This won't work in older browsers, but ok. > BugsSite/code-review.js:289 > + // Can't append the HTML because we need to set the onload handler before appending the iframe to the DOM. > + statusBubble.onload = handleStatusBubbleLoad; You could use a live event handler, but this seems fine. 322908 4 ojan 2010-12-15 16:03:00 -0800 Committed r74160: <http://trac.webkit.org/changeset/74160> 323743 5 ossy 2010-12-17 06:24:06 -0800 Reopen, because it is buggy: After this patch, code review page doesn't work for me with Firefox, I see the pretty diff, but I can't comment the source and I can't see the cq, review flags. I tried with Chrome too. It works, but the 3 lines height textarea of the overall comment is so ugly. 323773 6 ojan 2010-12-17 08:04:37 -0800 (In reply to comment #5) > After this patch, code review page doesn't work for me with Firefox, > I see the pretty diff, but I can't comment the source and I can't > see the cq, review flags. I'll try to fix this this morning, but I leave on vacation in a couple hours. I didn't realize anyone on the WebKit project did code reviews in FF. If I don't get it in by then, it will be about 2 weeks before I can get back to it. > I tried with Chrome too. It works, but the 3 lines > height textarea of the overall comment is so ugly. It's hard to work with "ugly" as feedback. Patches are welcome. 323793 7 tonikitoo 2010-12-17 08:32:58 -0800 (In reply to comment #6) > (In reply to comment #5) > > After this patch, code review page doesn't work for me with Firefox, > > I see the pretty diff, but I can't comment the source and I can't > > see the cq, review flags. > > I'll try to fix this this morning, but I leave on vacation in a couple hours. I didn't realize anyone on the WebKit project did code reviews in FF. If I don't get it in by then, it will be about 2 weeks before I can get back to it. I do reviewed on either Firefox or Chromium or Safari, whatever is easily accessible :) > > I tried with Chrome too. It works, but the 3 lines > > height textarea of the overall comment is so ugly. I also agree here, Ojan :( ... Not sure what I could feedback to you other than a visual adjective. 323863 8 ojan 2010-12-17 10:41:48 -0800 OK. It's now functional in Firefox after http://trac.webkit.org/changeset/74276 and http://trac.webkit.org/changeset/74277. It's not pretty though. I assume it must be a difference between webkit's and Firefox's flexbox implementations. Not planning on working on it more unless there's significant demand for this tool to work well in Firefox. Happy to do reviews for it though. :) It's hard to develop on the tool for Firefox because the way we do so is via a Chrome extension. Hope this is good enough. 76692 2010-12-15 15:17:32 -0800 2010-12-15 15:18:22 -0800 Patch bug-51142-20101215151731.patch text/plain 2885 ojan ZGlmZiAtLWdpdCBhL0J1Z3NTaXRlL0NoYW5nZUxvZyBiL0J1Z3NTaXRlL0NoYW5nZUxvZwppbmRl eCA3NWZhNTc2YTcyODBjZmMxYTJhYTVmMDIwMGE4OWQ5ZDRiNzdlNGQ1Li5mNjRhMzVhNzEyMDU0 NjkzNzUwNjg3NmU1YjhkYTVlMTdiYWUzYTAwIDEwMDY0NAotLS0gYS9CdWdzU2l0ZS9DaGFuZ2VM b2cKKysrIGIvQnVnc1NpdGUvQ2hhbmdlTG9nCkBAIC0xLDMgKzEsMTMgQEAKKzIwMTAtMTItMTUg IE9qYW4gVmFmYWkgIDxvamFuQGNocm9taXVtLm9yZz4KKworICAgICAgICBSZXZpZXdlZCBieSBO T0JPRFkgKE9PUFMhKS4KKworICAgICAgICBzaXplIHN0YXR1cyBidWJibGUgdG8gaXQncyBjb250 ZW50cyBvbiB0aGUgY29kZSByZXZpZXcgcGFnZQorICAgICAgICBodHRwczovL2J1Z3Mud2Via2l0 Lm9yZy9zaG93X2J1Zy5jZ2k/aWQ9NTExNDIKKworICAgICAgICAqIFByZXR0eVBhdGNoL1ByZXR0 eVBhdGNoLnJiOgorICAgICAgICAqIGNvZGUtcmV2aWV3LmpzOgorCiAyMDEwLTEyLTE0ICBPamFu IFZhZmFpICA8b2phbkBjaHJvbWl1bS5vcmc+CiAKICAgICAgICAgUmV2aWV3ZWQgYnkgQWRhbSBC YXJ0aC4KZGlmZiAtLWdpdCBhL0J1Z3NTaXRlL1ByZXR0eVBhdGNoL1ByZXR0eVBhdGNoLnJiIGIv QnVnc1NpdGUvUHJldHR5UGF0Y2gvUHJldHR5UGF0Y2gucmIKaW5kZXggNzI0N2M3M2JjNGNkMWJl NmEwZTAxMGIxM2Y4MTczYzJlMGRmMWQ2MS4uYWQ0ZDBiMWU5ZTVkY2ZjYjAyYWM4ZjRmMmI2MmY5 NmM1Y2QwNDI5MCAxMDA2NDQKLS0tIGEvQnVnc1NpdGUvUHJldHR5UGF0Y2gvUHJldHR5UGF0Y2gu cmIKKysrIGIvQnVnc1NpdGUvUHJldHR5UGF0Y2gvUHJldHR5UGF0Y2gucmIKQEAgLTM1MSw3ICsz NTEsNyBAQCBib2R5IHsKIH0KIDwvc3R5bGU+CiA8c2NyaXB0IHNyYz0iaHR0cHM6Ly9hamF4Lmdv b2dsZWFwaXMuY29tL2FqYXgvbGlicy9qcXVlcnkvMS40LjIvanF1ZXJ5Lm1pbi5qcyI+PC9zY3Jp cHQ+IAotPHNjcmlwdCBzcmM9ImNvZGUtcmV2aWV3LmpzP3ZlcnNpb249MTQiPjwvc2NyaXB0PiAK KzxzY3JpcHQgc3JjPSJjb2RlLXJldmlldy5qcz92ZXJzaW9uPTE1Ij48L3NjcmlwdD4KIEVPRgog CiAgICAgZGVmIHNlbGYucmV2aXNpb25PckRlc2NyaXB0aW9uKHN0cmluZykKZGlmZiAtLWdpdCBh L0J1Z3NTaXRlL2NvZGUtcmV2aWV3LmpzIGIvQnVnc1NpdGUvY29kZS1yZXZpZXcuanMKaW5kZXgg ZGI3N2I1ZmRiZDk1Zjg5ZWM5MjI5NjAwODFkZDNjODMxMTU3ZDY2Zi4uOWNlNGQ3YzE5NWM3Nzg3 OWJmYTk1ZWQzYWJkMzE4N2RhNzEyNzY5OSAxMDA2NDQKLS0tIGEvQnVnc1NpdGUvY29kZS1yZXZp ZXcuanMKKysrIGIvQnVnc1NpdGUvY29kZS1yZXZpZXcuanMKQEAgLTI1MSw2ICsyNTEsMTggQEAK ICAgICB9KTsKICAgfQogCisgIHdpbmRvdy5hZGRFdmVudExpc3RlbmVyKCdtZXNzYWdlJywgZnVu Y3Rpb24oZSkgeworICAgIGlmIChlLm9yaWdpbiAhPSAnaHR0cHM6Ly93ZWJraXQtY29tbWl0LXF1 ZXVlLmFwcHNwb3QuY29tJykKKyAgICAgIHJldHVybjsKKworICAgICQoJy5zdGF0dXNCdWJibGUn KVswXS5zdHlsZS5oZWlnaHQgPSBlLmRhdGEuaGVpZ2h0OworICAgICQoJy5zdGF0dXNCdWJibGUn KVswXS5zdHlsZS53aWR0aCA9IGUuZGF0YS53aWR0aDsKKyAgfSk7CisKKyAgZnVuY3Rpb24gaGFu ZGxlU3RhdHVzQnViYmxlTG9hZChlKSB7CisgICAgZS50YXJnZXQuY29udGVudFdpbmRvdy5wb3N0 TWVzc2FnZSgnY29udGFpbmVyTWV0cmljcycsICdodHRwczovL3dlYmtpdC1jb21taXQtcXVldWUu YXBwc3BvdC5jb20nKTsKKyAgfQorCiAgIGZ1bmN0aW9uIGZldGNoSGlzdG9yeSgpIHsKICAgICAk LmdldCgnYXR0YWNobWVudC5jZ2k/aWQ9JyArIGF0dGFjaG1lbnRfaWQgKyAnJmFjdGlvbj1lZGl0 JywgZnVuY3Rpb24oZGF0YSkgewogICAgICAgdmFyIGJ1Z19pZCA9IC9BdHRhY2htZW50IFxkKyBE ZXRhaWxzIGZvciBCdWcgKFxkKykvLmV4ZWMoZGF0YSlbMV07CkBAIC0yNjgsNyArMjgwLDE1IEBA CiAKICAgICAgIHZhciBkZXRhaWxzID0gJChkYXRhKTsKICAgICAgIGFkZEZsYWdzRm9yQXR0YWNo bWVudChkZXRhaWxzKTsKLSAgICAgICQoJyNzdGF0dXNCdWJibGVDb250YWluZXInKS5hcHBlbmQo JCgnPGlmcmFtZSBzdHlsZT0ibWFyZ2luLXRvcDoycHg7IiBjbGFzcz0ic3RhdHVzQnViYmxlIiBz cmM9Imh0dHBzOi8vd2Via2l0LWNvbW1pdC1xdWV1ZS5hcHBzcG90LmNvbS9zdGF0dXMtYnViYmxl LycgKyBhdHRhY2htZW50X2lkICsgJyIgc2Nyb2xsaW5nPSJubyI+PC9pZnJhbWU+JykpOworCisg ICAgICB2YXIgc3RhdHVzQnViYmxlID0gZG9jdW1lbnQuY3JlYXRlRWxlbWVudCgnaWZyYW1lJyk7 CisgICAgICBzdGF0dXNCdWJibGUuY2xhc3NOYW1lID0gJ3N0YXR1c0J1YmJsZSc7CisgICAgICBz dGF0dXNCdWJibGUuc3JjICA9ICdodHRwczovL3dlYmtpdC1jb21taXQtcXVldWUuYXBwc3BvdC5j b20vc3RhdHVzLWJ1YmJsZS8nICsgYXR0YWNobWVudF9pZDsKKyAgICAgIHN0YXR1c0J1YmJsZS5z Y3JvbGxpbmcgPSAnbm8nOworICAgICAgLy8gQ2FuJ3QgYXBwZW5kIHRoZSBIVE1MIGJlY2F1c2Ug d2UgbmVlZCB0byBzZXQgdGhlIG9ubG9hZCBoYW5kbGVyIGJlZm9yZSBhcHBlbmRpbmcgdGhlIGlm cmFtZSB0byB0aGUgRE9NLgorICAgICAgc3RhdHVzQnViYmxlLm9ubG9hZCA9IGhhbmRsZVN0YXR1 c0J1YmJsZUxvYWQ7CisgICAgICAkKCcjc3RhdHVzQnViYmxlQ29udGFpbmVyJykuYXBwZW5kKHN0 YXR1c0J1YmJsZSk7CisKICAgICAgICQoJyN0b29sYmFyIC5idWdMaW5rJykuaHRtbCgnPGEgaHJl Zj0iL3Nob3dfYnVnLmNnaT9pZD0nICsgYnVnX2lkICsgJyIgdGFyZ2V0PSJfYmxhbmsiPkJ1ZyAn ICsgYnVnX2lkICsgJzwvYT4nKTsKICAgICB9KTsKICAgfQo= 76693 2010-12-15 15:18:25 -0800 2010-12-15 15:23:38 -0800 Patch bug-51142-20101215151824.patch text/plain 3114 ojan ZGlmZiAtLWdpdCBhL0J1Z3NTaXRlL0NoYW5nZUxvZyBiL0J1Z3NTaXRlL0NoYW5nZUxvZwppbmRl eCA3NWZhNTc2YTcyODBjZmMxYTJhYTVmMDIwMGE4OWQ5ZDRiNzdlNGQ1Li5mNjRhMzVhNzEyMDU0 NjkzNzUwNjg3NmU1YjhkYTVlMTdiYWUzYTAwIDEwMDY0NAotLS0gYS9CdWdzU2l0ZS9DaGFuZ2VM b2cKKysrIGIvQnVnc1NpdGUvQ2hhbmdlTG9nCkBAIC0xLDMgKzEsMTMgQEAKKzIwMTAtMTItMTUg IE9qYW4gVmFmYWkgIDxvamFuQGNocm9taXVtLm9yZz4KKworICAgICAgICBSZXZpZXdlZCBieSBO T0JPRFkgKE9PUFMhKS4KKworICAgICAgICBzaXplIHN0YXR1cyBidWJibGUgdG8gaXQncyBjb250 ZW50cyBvbiB0aGUgY29kZSByZXZpZXcgcGFnZQorICAgICAgICBodHRwczovL2J1Z3Mud2Via2l0 Lm9yZy9zaG93X2J1Zy5jZ2k/aWQ9NTExNDIKKworICAgICAgICAqIFByZXR0eVBhdGNoL1ByZXR0 eVBhdGNoLnJiOgorICAgICAgICAqIGNvZGUtcmV2aWV3LmpzOgorCiAyMDEwLTEyLTE0ICBPamFu IFZhZmFpICA8b2phbkBjaHJvbWl1bS5vcmc+CiAKICAgICAgICAgUmV2aWV3ZWQgYnkgQWRhbSBC YXJ0aC4KZGlmZiAtLWdpdCBhL0J1Z3NTaXRlL1ByZXR0eVBhdGNoL1ByZXR0eVBhdGNoLnJiIGIv QnVnc1NpdGUvUHJldHR5UGF0Y2gvUHJldHR5UGF0Y2gucmIKaW5kZXggNzI0N2M3M2JjNGNkMWJl NmEwZTAxMGIxM2Y4MTczYzJlMGRmMWQ2MS4uN2U2NTkxZDY1YTA2NzY4YTg2ZjNiN2Q2YWI5YzQy NjljYzY1MDkxYiAxMDA2NDQKLS0tIGEvQnVnc1NpdGUvUHJldHR5UGF0Y2gvUHJldHR5UGF0Y2gu cmIKKysrIGIvQnVnc1NpdGUvUHJldHR5UGF0Y2gvUHJldHR5UGF0Y2gucmIKQEAgLTM0MiwxNiAr MzQyLDEzIEBAIGJvZHkgewogfQogCiAuc3RhdHVzQnViYmxlIHsKLSAgbWFyZ2luLXRvcDogMnB4 OwotICAvKiBGSVhNRTogU2l6ZSB0aGUgc3RhdHVzQnViYmxlIHZpYSBwb3N0TWVzc2FnZSBzbyBp dCBzaXplcyB0byBpdCdzIGNvbnRlbnQuICovCi0gIHdpZHRoOiAzMDBweDsKLSAgaGVpZ2h0OiAy MHB4OworICBtYXJnaW46IDJweCAycHggMCAwOwogICBib3JkZXI6IG5vbmU7CiAgIHZlcnRpY2Fs LWFsaWduOiBtaWRkbGU7CiB9CiA8L3N0eWxlPgogPHNjcmlwdCBzcmM9Imh0dHBzOi8vYWpheC5n b29nbGVhcGlzLmNvbS9hamF4L2xpYnMvanF1ZXJ5LzEuNC4yL2pxdWVyeS5taW4uanMiPjwvc2Ny aXB0PiAKLTxzY3JpcHQgc3JjPSJjb2RlLXJldmlldy5qcz92ZXJzaW9uPTE0Ij48L3NjcmlwdD4g Cis8c2NyaXB0IHNyYz0iY29kZS1yZXZpZXcuanM/dmVyc2lvbj0xNSI+PC9zY3JpcHQ+CiBFT0YK IAogICAgIGRlZiBzZWxmLnJldmlzaW9uT3JEZXNjcmlwdGlvbihzdHJpbmcpCmRpZmYgLS1naXQg YS9CdWdzU2l0ZS9jb2RlLXJldmlldy5qcyBiL0J1Z3NTaXRlL2NvZGUtcmV2aWV3LmpzCmluZGV4 IGRiNzdiNWZkYmQ5NWY4OWVjOTIyOTYwMDgxZGQzYzgzMTE1N2Q2NmYuLjljZTRkN2MxOTVjNzc4 NzliZmE5NWVkM2FiZDMxODdkYTcxMjc2OTkgMTAwNjQ0Ci0tLSBhL0J1Z3NTaXRlL2NvZGUtcmV2 aWV3LmpzCisrKyBiL0J1Z3NTaXRlL2NvZGUtcmV2aWV3LmpzCkBAIC0yNTEsNiArMjUxLDE4IEBA CiAgICAgfSk7CiAgIH0KIAorICB3aW5kb3cuYWRkRXZlbnRMaXN0ZW5lcignbWVzc2FnZScsIGZ1 bmN0aW9uKGUpIHsKKyAgICBpZiAoZS5vcmlnaW4gIT0gJ2h0dHBzOi8vd2Via2l0LWNvbW1pdC1x dWV1ZS5hcHBzcG90LmNvbScpCisgICAgICByZXR1cm47CisKKyAgICAkKCcuc3RhdHVzQnViYmxl JylbMF0uc3R5bGUuaGVpZ2h0ID0gZS5kYXRhLmhlaWdodDsKKyAgICAkKCcuc3RhdHVzQnViYmxl JylbMF0uc3R5bGUud2lkdGggPSBlLmRhdGEud2lkdGg7CisgIH0pOworCisgIGZ1bmN0aW9uIGhh bmRsZVN0YXR1c0J1YmJsZUxvYWQoZSkgeworICAgIGUudGFyZ2V0LmNvbnRlbnRXaW5kb3cucG9z dE1lc3NhZ2UoJ2NvbnRhaW5lck1ldHJpY3MnLCAnaHR0cHM6Ly93ZWJraXQtY29tbWl0LXF1ZXVl LmFwcHNwb3QuY29tJyk7CisgIH0KKwogICBmdW5jdGlvbiBmZXRjaEhpc3RvcnkoKSB7CiAgICAg JC5nZXQoJ2F0dGFjaG1lbnQuY2dpP2lkPScgKyBhdHRhY2htZW50X2lkICsgJyZhY3Rpb249ZWRp dCcsIGZ1bmN0aW9uKGRhdGEpIHsKICAgICAgIHZhciBidWdfaWQgPSAvQXR0YWNobWVudCBcZCsg RGV0YWlscyBmb3IgQnVnIChcZCspLy5leGVjKGRhdGEpWzFdOwpAQCAtMjY4LDcgKzI4MCwxNSBA QAogCiAgICAgICB2YXIgZGV0YWlscyA9ICQoZGF0YSk7CiAgICAgICBhZGRGbGFnc0ZvckF0dGFj aG1lbnQoZGV0YWlscyk7Ci0gICAgICAkKCcjc3RhdHVzQnViYmxlQ29udGFpbmVyJykuYXBwZW5k KCQoJzxpZnJhbWUgc3R5bGU9Im1hcmdpbi10b3A6MnB4OyIgY2xhc3M9InN0YXR1c0J1YmJsZSIg c3JjPSJodHRwczovL3dlYmtpdC1jb21taXQtcXVldWUuYXBwc3BvdC5jb20vc3RhdHVzLWJ1YmJs ZS8nICsgYXR0YWNobWVudF9pZCArICciIHNjcm9sbGluZz0ibm8iPjwvaWZyYW1lPicpKTsKKwor ICAgICAgdmFyIHN0YXR1c0J1YmJsZSA9IGRvY3VtZW50LmNyZWF0ZUVsZW1lbnQoJ2lmcmFtZScp OworICAgICAgc3RhdHVzQnViYmxlLmNsYXNzTmFtZSA9ICdzdGF0dXNCdWJibGUnOworICAgICAg c3RhdHVzQnViYmxlLnNyYyAgPSAnaHR0cHM6Ly93ZWJraXQtY29tbWl0LXF1ZXVlLmFwcHNwb3Qu Y29tL3N0YXR1cy1idWJibGUvJyArIGF0dGFjaG1lbnRfaWQ7CisgICAgICBzdGF0dXNCdWJibGUu c2Nyb2xsaW5nID0gJ25vJzsKKyAgICAgIC8vIENhbid0IGFwcGVuZCB0aGUgSFRNTCBiZWNhdXNl IHdlIG5lZWQgdG8gc2V0IHRoZSBvbmxvYWQgaGFuZGxlciBiZWZvcmUgYXBwZW5kaW5nIHRoZSBp ZnJhbWUgdG8gdGhlIERPTS4KKyAgICAgIHN0YXR1c0J1YmJsZS5vbmxvYWQgPSBoYW5kbGVTdGF0 dXNCdWJibGVMb2FkOworICAgICAgJCgnI3N0YXR1c0J1YmJsZUNvbnRhaW5lcicpLmFwcGVuZChz dGF0dXNCdWJibGUpOworCiAgICAgICAkKCcjdG9vbGJhciAuYnVnTGluaycpLmh0bWwoJzxhIGhy ZWY9Ii9zaG93X2J1Zy5jZ2k/aWQ9JyArIGJ1Z19pZCArICciIHRhcmdldD0iX2JsYW5rIj5CdWcg JyArIGJ1Z19pZCArICc8L2E+Jyk7CiAgICAgfSk7CiAgIH0K