Opened 11 years ago

Closed 11 years ago

Last modified 11 years ago

#6921 closed defect (fixed)

[patch] dojo.coords() problems on Safari and Opera

Reported by: Eugene Lazutkin Owned by: haysmark
Priority: high Milestone: 1.2
Component: HTML Version: 1.1.1
Keywords: Cc:
Blocked By: Blocking:

Description

Tooltips look off when the page is scrolled on Safari and Opera (tested current versions + Opera 9.5b on Linux and Windows). Looking at the code I suspect that dojo.coords() is at fault.

I was able to reproduce the problem with dijit/tests/test_Tooltip.html. Scrolling the page it is easy to see that tooltips on some elements are off, yet other elements work as expected.

Attachments (3)

abs_wip.patch (2.0 KB) - added by haysmark 11 years ago.
Possible fix for dojo._abs.
abs_wip.2.patch (4.4 KB) - added by haysmark 11 years ago.
Another fix to dojo._abs, for wide borders.
abs_wip.3.patch (1.7 KB) - added by haysmark 11 years ago.
Fix for Opera and FF2. All abs tests pass with this patch.

Download all attachments as: .zip

Change History (17)

comment:1 Changed 11 years ago by bill

Strange... I see the deviant behavior on links and buttons.

comment:2 Changed 11 years ago by bill

Milestone: 1.2
Owner: changed from alex to bill
Status: newassigned

I'll fix. This is due to the checkin [12117] / #5295. ISTM safari flip-flopped it's behavior between 2.0 and 3.0 and then back again for 3.1 (or whenever it was that this problem started occurring).

comment:3 Changed 11 years ago by bill

I spoke too soon. The difference between the elements that appear correctly or not is whether those elements are direct children of <body>, and sure enough dojo._abs() has if() statements branching on that condition (although perhaps they are doing more harm than good).

comment:4 Changed 11 years ago by bill

(In [14223]) Supplementary tests for dojo._abs(). Unfortunately they seem to fail in one way or another on all browsers. This should be converted to a DOH automated test. Refs #6921.

Changed 11 years ago by haysmark

Attachment: abs_wip.patch added

Possible fix for dojo._abs.

comment:5 Changed 11 years ago by haysmark

Here is a diff that I have had better success with.

My experiences with the abs test comparing the nightly vs my patch:

IE: unchanged

FF2:

  • Nightly: Everything in at least one div was off (not adding computed margin/padding of parents?)
  • Patched: only click3 fails

FF3:

  • Nightly: Consistently placed red box in bottom-middle of button
  • Patched: clicks 3,7,8 appear just a tad too far inside the button

Safari 3.1.1:

  • Nightly: Very bottom button failed, very top button failed when you scrolled down a little
  • Patched: Everything works

Opera 9.27:

  • Nightly: Everything fails
  • Patched: 3,5,7,8 fail when you scroll their parent divs down; scrolling the document has no effect.

comment:6 Changed 11 years ago by bill

Thanks Mark, this looks great. I ran the tests also, got the same results as you. I will check in but leave the ticket open for the remaining issues.

comment:7 Changed 11 years ago by bill

Above patch checked in as [14326].

Changed 11 years ago by haysmark

Attachment: abs_wip.2.patch added

Another fix to dojo._abs, for wide borders.

comment:8 Changed 11 years ago by haysmark

Here is a new patch for dojo._abs.

I tried test_BorderContainer and noticed that my first patch broke in FF2 and FF3. FF3 was actually better off with the IE code but still needed a fix for the strange offset. The reason FF3 was offset was because you needed to add the margin of the document element. So now everything is much better in FF3 since I fixed the IE path with a FF3 conditional.

Even in Dojo 1.1 FF2 abs was just horribly broken and it became very obvious after I added 17px borders to the abs test and ran it against 1.1. My first patch only appeared to work because the abs test had thin borders. The padding/margin weren't the problem: the borders were.

With this new patch, FF2 passes the bordered abs test and is working for BorderContainer? again. click3 is still broken and now it is very obvious just how much so with the added borders. It looks like that because click3 is static AND is in a static div that has a border, click3 is being offset by the border. The code skips over this div since the div is not in the offsetParent chain. Hmm.

Safari and IE are unchanged with the addition of borders to the abs test and still pass. Opera still has the problems I mentioned earlier.

comment:9 Changed 11 years ago by Nathan Toone

With this most recent patch, I can verify that click3 is broken on FF2.

Click3, click5, click7, and click8 are still broken in Opera (9.51). However, since it's still better and fixes the FF3 issues as well, I'll check it in (but still leave open).

comment:10 Changed 11 years ago by Nathan Toone

(In [14339]) Refs #6921 - updated patch by haysmark to fix FF3 and FF2 issues. See ticket for details !strict

comment:11 Changed 11 years ago by bill

Owner: changed from bill to haysmark
Status: assignednew

I guess we are leaving this open until every single test works.

Changed 11 years ago by haysmark

Attachment: abs_wip.3.patch added

Fix for Opera and FF2. All abs tests pass with this patch.

comment:12 Changed 11 years ago by haysmark

Status: newassigned
Summary: dojo.coords() problems on Safari and Opera[patch] dojo.coords() problems on Safari and Opera

Here is another patch for dojo._abs.

I added a new code path for Opera regarding the scrolling. Apparently position:absolute has some interesting effects on how Opera calculates scrollLeft/Top. Now all buttons in dojo/tests/abs.html work for Opera.

Also, my suspicions about FF2 static elements seem to be correct. click3 now works in FF2, so now all buttons work for FF2.

comment:13 Changed 11 years ago by bill

Resolution: fixed
Status: assignedclosed

(In [14470]) Further patches to dojo._abs() for opera and FF2. Fixes #6921 !strict Patch from Mark Hays (IBM, CCLA on file). Thanks!

comment:14 Changed 11 years ago by bill

See also [15730] and [15731] for further changes.

Note: See TracTickets for help on using tickets.