12687 2007-02-07 16:52:13 -0800 REGRESSION: Google Calendar cell highlight misplaced 2007-02-11 16:31:29 -0800 1 1 1 Unclassified WebKit Layout and Rendering 420+ Mac OS X 10.4 RESOLVED FIXED http://www.google.com/calendar/render?pli=1 GoogleBug, NeedsReduction, Regression P1 Normal --- 1 koivisto webkit-unassigned abob hyatt mitz oldest_to_newest 24962 0 koivisto 2007-02-07 16:52:13 -0800 1. login to Google Calendar 2. click somewhere on main calendar area to select a time The grey transparent highlight is drawn shifted several pixels right/down from cell area where it should be. Works fine in shipping Safari. 25024 1 mitz 2007-02-07 22:27:37 -0800 Could be a result of changing offsetTop/offsetLeft behavior to match Firefox recently. 25038 2 mitz 2007-02-07 23:49:49 -0800 (In reply to comment #1) > Could be a result of changing offsetTop/offsetLeft behavior to match Firefox > recently. > Wrong. Based on nightly builds, the regression happened sometime between 2005-12-01 and 2005-12-16. 25042 3 mitz 2007-02-08 00:02:16 -0800 Perhaps this: <http://trac.webkit.org/projects/webkit/changeset/11484>. 25049 4 koivisto 2007-02-08 01:36:25 -0800 No, wasn't that one. Something else...(In reply to comment #3) > Perhaps this: <http://trac.webkit.org/projects/webkit/changeset/11484>. > No, doesn't seem to be that one. 25059 5 mitz 2007-02-08 04:20:24 -0800 WebKit-CVS-2005-12-07 10-27-14 GMT.dmg works WebKit-CVS-2005-12-08 23-04-18 GMT.dmg broken 24650 6 koivisto 2007-02-09 08:18:28 -0800 If you revert both Bug 11109 and http://trac.webkit.org/projects/webkit/changeset/11484 then you get correct positioning. This is because Calendar works around offsetLeft/offsetTop/offsetParent bug in tiger webkit. Specifically tiger webkit would not include body margins to offsets of relative positioned elements. This code in function qC(a, b, c, d) in the obfuscated calendar script if (T) i.x += 3; else if ($) { i.x += fb() ? 6 : 8; i.y += 6; } adds some magic offsets to fix positioning issues. For safari $=1, for MSIE T=1. Safari offsetLeft behavior is still not exactly the same as Firefox. For some reason, like MSIE, ToT Safari needs that i.x += 3; to get pixel-correct horizontal position. Is the idea here to try to replicate MSIE or Firefox behavior for offset functions? If target is MSIE then I don't think there is any bug here. 24651 7 mitz 2007-02-09 08:27:03 -0800 (In reply to comment #6) > Safari offsetLeft behavior is still not exactly the same as Firefox. > Is the idea here to try to replicate MSIE or Firefox > behavior for offset functions? My intention with the patch for bug 11109 was to replicate Firefox (WinIE behavior is described in one of my comments on that bug). Do you know what the remaining discrepancy is? 24655 8 13086 koivisto 2007-02-09 09:36:19 -0800 Created attachment 13086 test case showing different offset behavior compared to ie/ff Both FF and IE agree that offsetLeft/Top should be measured from inner border edge of the offsetParent. WebKit measures it from outer border edge of the parent. In this case first offsetLeft is zero in FF/IE and non-zero in Safari 24663 9 koivisto 2007-02-09 11:02:38 -0800 Here is a nice test suite for offset properties by Anne van Kesteren: http://dump.testsuite.org/2006/dom/style/offset/ We get quite a few wrong and more importantly we get quite a few different from FF. Especially offsetParent seems to be often wrong. 24668 10 ddkilzer 2007-02-09 11:19:24 -0800 Per Comment #6, it sounds like both Safari and Google Calendar will need fixes to resolve this bug. 24670 11 ddkilzer 2007-02-09 11:25:12 -0800 (In reply to comment #9) > Here is a nice test suite for offset properties by Anne van Kesteren: > > http://dump.testsuite.org/2006/dom/style/offset/ > > We get quite a few wrong and more importantly we get quite a few different from > FF. Especially offsetParent seems to be often wrong. Filed Bug 12713 to import this test suite. 24683 12 13091 koivisto 2007-02-09 13:26:37 -0800 Created attachment 13091 proposed patch Ok, so here is proposed patch. It does three things 1) calculate offsets from padding edge (inner edge of border), not from border edge 2) revert this !(!style()->htmlHacks() && skipTables ? curr->isRoot() : curr->isBody()) in http://trac.webkit.org/projects/webkit/changeset/11484 back to this !curr->isBody() which seems to match FF behavior in both strict and compat modes. Why was this change done? Hard to say since there was no test case nor explanation. 3) Check for tableness from dom tree instead of rendering tree. That's just what other browser seem to do based on Anne's tests. These changes make all Anne's tests pass except for 3,8,12,29,43,45,49,50. In all those cases we match FF behavior (except 12, where we fail a bit less). Failures are of course all debatable since there is no authorative spec about offset behaviors. No layout test failures either. (some of Anne's test cases may randomly produce wrong offsetTop results when reloaded. that is some layout timing problem not directly related to offset calculations) None of this actually fixes Google Calendar, it needs ToT fixes in the other end. Best we can do is to make this stuff consistent. 24640 13 abob 2007-02-09 14:12:33 -0800 This problem has been written to the Google bug tracking system. 631734 24435 14 13091 mjs 2007-02-10 16:21:36 -0800 Comment on attachment 13091 proposed patch Fix looks good (Mitz agrees) but needs ChangeLog entry and test case. Feel free to convert this to an r+ if you add a ChangeLog and land the attached test case w/ this. Also, this won't really make the bug on Google go away, but being more like FF will allow us to turn this into strictly an evangelism bug. 24437 15 13091 mjs 2007-02-10 16:27:40 -0800 Comment on attachment 13091 proposed patch r- per above comments 24112 16 koivisto 2007-02-11 16:29:17 -0800 Commited as r19567 with attached layout test based on comment #14 above. 24113 17 13091 koivisto 2007-02-11 16:31:29 -0800 Comment on attachment 13091 proposed patch see comment 14 13086 2007-02-09 09:36:19 -0800 2007-02-09 09:36:19 -0800 test case showing different offset behavior compared to ie/ff 12687.html text/html 449 koivisto PGh0bWw+CjxoZWFkPgoKPGJvZHkgc3R5bGU9J21hcmdpbjo4cHgnPgo8ZGl2Pgo8ZGl2IHN0eWxl PSJwb3NpdGlvbjogYWJzb2x1dGU7IHdpZHRoOiAxNTBweDsgaGVpZ2h0OiAxNTBweDsgYm9yZGVy OiA1cHggc29saWQgZ3JlZW4iPgo8ZGl2IGlkPSd4JyBzdHlsZT0icG9zaXRpb246IGFic29sdXRl OyBsZWZ0OjAlOyB0b3A6MCU7IHdpZHRoOiA1MHB4OyBoZWlnaHQ6IDUwcHg7IGJvcmRlcjoxM3B4 IHNvbGlkIGJsdWUiPgo8L2Rpdj4KPC9kaXY+CjxzY3JpcHQ+CnZhciBlbCA9IGRvY3VtZW50Lmdl dEVsZW1lbnRCeUlkKCd4Jyk7CndoaWxlIChlbCkgewpkb2N1bWVudC53cml0ZSgnPGJyPi0tLS0t LS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS1vZmZzZXRMZWZ0Oicr ZWwub2Zmc2V0TGVmdCk7CmVsID0gZWwub2Zmc2V0UGFyZW50Owp9Cjwvc2NyaXB0Pgo= 13091 2007-02-09 13:26:37 -0800 2007-02-11 16:31:29 -0800 proposed patch 12687.patch text/plain 1759 koivisto SW5kZXg6IFdlYkNvcmUvcmVuZGVyaW5nL1JlbmRlck9iamVjdC5jcHAKPT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PQotLS0g V2ViQ29yZS9yZW5kZXJpbmcvUmVuZGVyT2JqZWN0LmNwcAkocmV2aXNpb24gMTk0ODEpCisrKyBX ZWJDb3JlL3JlbmRlcmluZy9SZW5kZXJPYmplY3QuY3BwCSh3b3JraW5nIGNvcHkpCkBAIC01MDcs NyArNTA3LDcgQEAgaW50IFJlbmRlck9iamVjdDo6b2Zmc2V0TGVmdCgpIGNvbnN0CiAgICAgUmVu ZGVyT2JqZWN0KiBvZmZzZXRQYXIgPSBvZmZzZXRQYXJlbnQoKTsKICAgICBpZiAoIW9mZnNldFBh cikKICAgICAgICAgcmV0dXJuIDA7Ci0gICAgaW50IHggPSB4UG9zKCk7CisgICAgaW50IHggPSB4 UG9zKCkgLSBvZmZzZXRQYXItPmJvcmRlckxlZnQoKTsKICAgICBpZiAoIWlzUG9zaXRpb25lZCgp KSB7CiAgICAgICAgIGlmIChpc1JlbFBvc2l0aW9uZWQoKSkKICAgICAgICAgICAgIHggKz0gc3Rh dGljX2Nhc3Q8Y29uc3QgUmVuZGVyQm94Kj4odGhpcyktPnJlbGF0aXZlUG9zaXRpb25PZmZzZXRY KCk7CkBAIC01MjcsNyArNTI3LDcgQEAgaW50IFJlbmRlck9iamVjdDo6b2Zmc2V0VG9wKCkgY29u c3QKICAgICBSZW5kZXJPYmplY3QqIG9mZnNldFBhciA9IG9mZnNldFBhcmVudCgpOwogICAgIGlm ICghb2Zmc2V0UGFyKQogICAgICAgICByZXR1cm4gMDsKLSAgICBpbnQgeSA9IHlQb3MoKTsKKyAg ICBpbnQgeSA9IHlQb3MoKSAtIG9mZnNldFBhci0+Ym9yZGVyVG9wKCk7CiAgICAgaWYgKCFpc1Bv c2l0aW9uZWQoKSkgewogICAgICAgICBpZiAoaXNSZWxQb3NpdGlvbmVkKCkpCiAgICAgICAgICAg ICB5ICs9IHN0YXRpY19jYXN0PGNvbnN0IFJlbmRlckJveCo+KHRoaXMpLT5yZWxhdGl2ZVBvc2l0 aW9uT2Zmc2V0WSgpOwpAQCAtNTUyLDkgKzU1Miw5IEBAIFJlbmRlck9iamVjdCogUmVuZGVyT2Jq ZWN0OjpvZmZzZXRQYXJlbnQKICAgICBib29sIHNraXBUYWJsZXMgPSBpc1Bvc2l0aW9uZWQoKSB8 fCBpc1JlbFBvc2l0aW9uZWQoKTsKICAgICBSZW5kZXJPYmplY3QqIGN1cnIgPSBwYXJlbnQoKTsK ICAgICB3aGlsZSAoY3VyciAmJiAoIWN1cnItPmVsZW1lbnQoKSB8fAotICAgICAgICAgICAgICAg ICAgICAoIWN1cnItPmlzUG9zaXRpb25lZCgpICYmICFjdXJyLT5pc1JlbFBvc2l0aW9uZWQoKSAm JgotICAgICAgICAgICAgICAgICAgICAgICAgISghc3R5bGUoKS0+aHRtbEhhY2tzKCkgJiYgc2tp cFRhYmxlcyA/IGN1cnItPmlzUm9vdCgpIDogY3Vyci0+aXNCb2R5KCkpKSkpIHsKLSAgICAgICAg aWYgKCFza2lwVGFibGVzICYmIGN1cnItPmVsZW1lbnQoKSAmJiAoY3Vyci0+aXNUYWJsZUNlbGwo KSB8fCBjdXJyLT5pc1RhYmxlKCkpKQorICAgICAgICAgICAgICAgICAgICAoIWN1cnItPmlzUG9z aXRpb25lZCgpICYmICFjdXJyLT5pc1JlbFBvc2l0aW9uZWQoKSAmJiAhY3Vyci0+aXNCb2R5KCkp KSkgeworICAgICAgICBpZiAoIXNraXBUYWJsZXMgJiYgY3Vyci0+ZWxlbWVudCgpICYmIChjdXJy LT5lbGVtZW50KCktPmhhc1RhZ05hbWUodGFibGVUYWcpIHx8IAorICAgICAgICAgICAgICAgICAg ICAgICAgICAgICAgICAgICAgICAgICAgICAgICBjdXJyLT5lbGVtZW50KCktPmhhc1RhZ05hbWUo dGRUYWcpIHx8IGN1cnItPmVsZW1lbnQoKS0+aGFzVGFnTmFtZSh0aFRhZykpKQogICAgICAgICAg ICAgYnJlYWs7CiAgICAgICAgIGN1cnIgPSBjdXJyLT5wYXJlbnQoKTsKICAgICB9Cg==