WebKit Bugzilla
New
Browse
Log In
×
Sign in with GitHub
or
Remember my login
Create Account
·
Forgot Password
Forgotten password account recovery
RESOLVED FIXED
12687
REGRESSION: Google Calendar cell highlight misplaced
https://bugs.webkit.org/show_bug.cgi?id=12687
Summary
REGRESSION: Google Calendar cell highlight misplaced
Antti Koivisto
Reported
2007-02-07 16:52:13 PST
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.
Attachments
test case showing different offset behavior compared to ie/ff
(449 bytes, text/html)
2007-02-09 09:36 PST
,
Antti Koivisto
no flags
Details
proposed patch
(1.72 KB, patch)
2007-02-09 13:26 PST
,
Antti Koivisto
koivisto
: review+
Details
Formatted Diff
Diff
View All
Add attachment
proposed patch, testcase, etc.
mitz
Comment 1
2007-02-07 22:27:37 PST
Could be a result of changing offsetTop/offsetLeft behavior to match Firefox recently.
mitz
Comment 2
2007-02-07 23:49:49 PST
(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.
mitz
Comment 3
2007-02-08 00:02:16 PST
Perhaps this: <
http://trac.webkit.org/projects/webkit/changeset/11484
>.
Antti Koivisto
Comment 4
2007-02-08 01:36:25 PST
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.
mitz
Comment 5
2007-02-08 04:20:24 PST
WebKit-CVS-2005-12-07 10-27-14 GMT.dmg works WebKit-CVS-2005-12-08 23-04-18 GMT.dmg broken
Antti Koivisto
Comment 6
2007-02-09 08:18:28 PST
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.
mitz
Comment 7
2007-02-09 08:27:03 PST
(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?
Antti Koivisto
Comment 8
2007-02-09 09:36:19 PST
Created
attachment 13086
[details]
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
Antti Koivisto
Comment 9
2007-02-09 11:02:38 PST
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.
David Kilzer (:ddkilzer)
Comment 10
2007-02-09 11:19:24 PST
Per
Comment #6
, it sounds like both Safari and Google Calendar will need fixes to resolve this bug.
David Kilzer (:ddkilzer)
Comment 11
2007-02-09 11:25:12 PST
(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.
Antti Koivisto
Comment 12
2007-02-09 13:26:37 PST
Created
attachment 13091
[details]
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.
Bob Austin
Comment 13
2007-02-09 14:12:33 PST
This problem has been written to the Google bug tracking system. 631734
Maciej Stachowiak
Comment 14
2007-02-10 16:21:36 PST
Comment on
attachment 13091
[details]
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.
Maciej Stachowiak
Comment 15
2007-02-10 16:27:40 PST
Comment on
attachment 13091
[details]
proposed patch r- per above comments
Antti Koivisto
Comment 16
2007-02-11 16:29:17 PST
Commited as
r19567
with attached layout test based on
comment #14
above.
Antti Koivisto
Comment 17
2007-02-11 16:31:29 PST
Comment on
attachment 13091
[details]
proposed patch see
comment 14
Note
You need to
log in
before you can comment on or make changes to this bug.
Top of Page
Format For Printing
XML
Clone This Bug