Opened 9 years ago

Closed 9 years ago

#11376 closed defect (fixed)

dojo.position(node, true) problem on IE8 standards RTL mode

Reported by: bill Owned by: Douglas Hays
Priority: high Milestone: 1.6
Component: HTML Version: 1.5.0b2
Keywords: Cc: James Burke
Blocked By: Blocking:

Description (last modified by bill)

In _autoComplete.html, add

<nowrap>ABCEFGHIJKLMONPQRSTUVWXYZABCEFGHIJKLMONPQRSTUVWXYZABCEFGHIJKLMONPQRSTUVWXYZABCEFGHIJKLMONPQRSTUVWXYZABCEFGHIJKLMONPQRSTUVWXYZABCEFGHIJKLMONPQRSTUVWXYZABCEFGHIJKLMONPQRSTUVWXYZABCEFGHIJKLMONPQRSTUVWXYZ</nowrap>

so that a horizontal scrollbar appears.

Then bring up test_ComboBox.html?dir=rtl in IE8.

From IE8's console run this command with the horizontal scrollbar at various positions:

dojo.position("setvaluetest", true).x

Notice how the value changes as you move the horizontal scrollbar. It shouldn't change.

This problem is causing the ComboBox drop down to be misaligned (on IE8, RTL, when scrolled), see #11364.

Attachments (1)

11376.patch (7.8 KB) - added by Douglas Hays 9 years ago.
fix to _docScroll for IE8 and IE/quirks, and additional testcases

Download all attachments as: .zip

Change History (10)

comment:1 Changed 9 years ago by bill

(In [22393]) Fix ComboBox drop down positioning problem when:

  1. the drop down's width has been increased to match the width of the ComboBox
  2. the page has a horizontal scrollbar, and has been scrolled

There's still an issue on IE8 in RTL mode though, listed in #11376.

Fixes #11364, refs #10581, #11376.

comment:2 Changed 9 years ago by bill

Description: modified (diff)

comment:3 Changed 9 years ago by Douglas Hays

Cc: James Burke added

dojo._fixIeBiDiScrollLeft is returning a positive number instead of -scrollLeft for IE8/RTL. The fix will ripple thru everyone who uses _docScroll.

comment:4 Changed 9 years ago by Douglas Hays

Milestone: tbd1.6

comment:5 Changed 9 years ago by Douglas Hays

Status: newassigned

James/Bill?, please review the attached patch. I included tests for combinations of strict and quirks, and ltr and rtl. _docScroll was not calling _fixIeBiDiScrollLeft in quirks mode, so it could not work for IE/RTL/quirks. For IE8/RTL/strict, _fixIeBiDiScrollLeft was not negating the scrollLeft and thus position's returned x value was off. The new tests that I added scroll the document both left and right, and up and down, and then use dojo.position'x x/y to place position:absolute elements overtop a normal static element, and then check to make sure each element has the same offsetLeft and offsetTop.

Changed 9 years ago by Douglas Hays

Attachment: 11376.patch added

fix to _docScroll for IE8 and IE/quirks, and additional testcases

comment:6 Changed 9 years ago by bill

I checked it against the original test case (about ComboBox). Seems to be working better than before, anyway. I noticed a few problems with drop down positioning but I think they are unrelated; I filed them as #11461.

I found dojo._docScroll confusing:

dojo._docScroll = function(){
	var n=d.global;
	return "pageXOffset" in n
		? { x:n.pageXOffset, y:n.pageYOffset }
		: (n=d.isQuirks? d.doc.body : d.doc.documentElement, { x:d._fixIeBiDiScrollLeft(n.scrollLeft||0), y:n.scrollTop||0 });
};

The n=d.isQuirks ? ... looks like a typo where it should be == but eventually I realized that (apparently) the ? has higher precedence, so it's effectively doing n = (d.isQuirks ? d.doc.body : d.doc.documentElement). The other thing that confused me was repurposing the "n" variable, rather than creating a new variable; admittedly that was also confusing in the original code.

I guess all that is in the name of saving a few bytes though.

One final thing: dojo.isQuirks will be true on all browsers, not just IE. But apparently that doesn't matter because non-IE browsers define pageXOffset and so take the first path, not even hitting the quirks calculation.

comment:7 Changed 9 years ago by Douglas Hays

Resolution: fixed
Status: assignedclosed

(In [22540]) Fixes #11376 !strict. Update _docScroll to also call _fixIeBiDiScrollLeft in quirks mode. Update _fixIeBiDiScrollLeft to negate scrollLeft in IE8/RTL mode. Added unit tests to ensure dojo.position(node, true) returns a value that can be used to absolute position a node in any of quirks/strict/rtl/ltr modes.

comment:8 Changed 9 years ago by bill

Resolution: fixed
Status: closedreopened

Unfortunately I'm getting errors in tests/_base/html.html on Safari 5.0.2/mac after the above change.

doh._AssertFailure: assertEqual() failed:
	expected
		100
	but got
		-47

	ERROR IN:
		 function coordsBasic(t) {
							var pos = dojo.position("sq100", false);
							// console.debug(pos);
							doh.is(100, pos.x);
							doh.is(100, pos.y);
							doh.is(100, pos.w);
							doh.is(100, pos.h);
						}
FAILED test: coordsBasic 1 ms
doh._AssertFailure: assertEqual() failed:
	expected
		110
	but got
		-37

	ERROR IN:
		 function coordsMargin(t) {
							// position() is getting us the border-box location
							var pos = dojo.position("sq100margin10", false);
							doh.is(260, pos.x);
							doh.is(110, pos.y);
							doh.is(100, pos.w);
							doh.is(100, pos.h);
							pos = dojo.marginBox("sq100margin10");
							doh.is(120, pos.w);
							doh.is(120, pos.h);
							// Though coords shouldn't be used, test it for backward compatibility.
							// coords returns the border-box location and margin-box size
							pos = dojo.coords("sq100margin10", false);
							doh.is(260, pos.x);
							doh.is(110, pos.y);
							doh.is(120, pos.w);
							doh.is(120, pos.h);
						}
FAILED test: coordsMargin 0 ms
doh._AssertFailure: assertEqual() failed:
	expected
		400
	but got
		253

	ERROR IN:
		 function coordsBorder(t) {
							var pos = dojo.position("sq100border10", false);
							doh.is(100, pos.x);
							doh.is(400, pos.y);
						}
FAILED test: coordsBorder 1 ms
	_AssertFailure: doh._AssertFailure: assertTrue('false') failed: assertTrue('false') failed
doh._AssertFailure: assertTrue('false') failed
	ERROR IN:
		 function sq100nopos(t) {
							var pos = dojo.position("sq100nopos", false);
							// console.debug(pos);
							doh.is(0, pos.x);
							doh.t(pos.y > 0);
							// FIXME: the 'correct' w is not 100 on Safari WebKit (2.0.4 [419.3]), the right-margin extends to the document edge
							// doh.is(100, pos.w);
							doh.is(100, pos.h);
						}
FAILED test: sq100nopos 1 ms

I'm also getting an error in html_rtl, probably from this change too although I didn't test it:

doh._AssertFailure: assertEqual() failed:
	expected
		50
	but got
		-1304

 with hint: 
		y pos should be 50 after vertical scroll
	ERROR IN:
		 function coordsWithVertScrollbar(t) {
							// show vertical scrollbar
							dojo.byId("rect_vert").style.display = "";
							scrollTo(0, 50);
							var d = new doh.Deferred();
							setTimeout(function(){ // need time to render
								try{
									t.is(100, dojo.position('rect100').x, "x pos should be 100 after vertical scroll");
									t.is(50, dojo.position('rect100').y, "y pos should be 50 after vertical scroll");
									d.callback(true);
								}catch(e){
									d.errback(e);
								}finally{
									dojo.byId("rect_vert").style.display = "none";
								}
							}, 100);
							return d;
						}
FAILED test: coordsWithVertScrollbar 102 ms

PS: I'm finding safari resistant to "empty cache", I had to switch from my trunk/ URL to my 1.5/ URL before calling "empty cache" in order to get rid of everything that was cached from trunk/.

comment:9 Changed 9 years ago by Douglas Hays

Resolution: fixed
Status: reopenedclosed

(In [22991]) Fixes #11376. Workaround safari scrolling/timing issues in the testcases.

Note: See TracTickets for help on using tickets.