Merge pull request #2136 from danzel/rtl

Correct rtl again
Leaflet · Nov 3, 2013 · 90ce2ab · 90ce2ab
2 parents 0ecdcd7 + 80c48c5
commit 90ce2ab
Show file tree

Hide file tree

Showing 6 changed files with 81 additions and 39 deletions.
diff --git a/debug/tests/rtl.html b/debug/tests/rtl.html
@@ -0,0 +1,43 @@
+<!DOCTYPE html>
+<html>
+<head>
+	<title>Leaflet debug page</title>
+
+	<link rel="stylesheet" href="../../dist/leaflet.css" />
+	<!--[if lte IE 8]><link rel="stylesheet" href="../../dist/leaflet.ie.css" /><![endif]-->
+
+	<meta name="viewport" content="width=device-width, initial-scale=1.0">
+
+	<link rel="stylesheet" href="../css/screen.css" />
+
+	<script type="text/javascript" src="../../build/deps.js"></script>
+	<script src="../leaflet-include.js"></script>
+	<style>
+	body {
+		direction: rtl;
+	}
+</style>
+</head>
+<body>
+
+	<p>Click the map to place a popup at the mouse location</p>
+	<div id="map"></div>
+
+	<script type="text/javascript">
+
+		var cloudmade = L.tileLayer('http://{s}.tile.cloudmade.com/{key}/997/256/{z}/{x}/{y}.png', {
+			maxZoom: 18,
+			attribution: 'Map data &copy; 2011 OpenStreetMap contributors, Imagery &copy; 2011 CloudMade',
+			key: 'd4fc77ea4a63471cab2423e66626cbb6'
+		});
+
+		var map = L.map('map')
+				.setView([50.5, 30.51], 15)
+				.addLayer(cloudmade);
+
+		map.on('click', function(e) {
+			L.popup().setLatLng(e.latlng).setContent('Hello').openOn(map);
+		});
+	</script>
+</body>
+</html>
diff --git a/debug/tests/rtl2.html b/debug/tests/rtl2.html
@@ -0,0 +1,28 @@
+<html dir="rtl">
+<head>
+
+	<link rel="stylesheet" href="../../dist/leaflet.css" />
+	<!--[if lte IE 8]><link rel="stylesheet" href="../../dist/leaflet.ie.css" /><![endif]-->
+
+	<meta name="viewport" content="width=device-width, initial-scale=1.0">
+
+	<link rel="stylesheet" href="../css/mobile.css" />
+
+	<script type="text/javascript" src="../../build/deps.js"></script>
+	<script src="../leaflet-include.js"></script>
+	<style>
+    #map { height: 100%; }
+  </style>
+</head>
+<body>
+<div id="map"></div>
+<script>
+  var map = L.map('map').setView([51.505, -0.09], 13);
+  L.tileLayer('http://{s}.tile.openstreetmap.org/{z}/{x}/{y}.png').addTo(map);
+map.on('click', function(e) {
+    L.popup().setLatLng(e.latlng).setContent('Hello').openOn(map);
+});
+
+</script>
+</body>
+</html>
diff --git a/spec/suites/dom/DomUtilSpec.js b/spec/suites/dom/DomUtilSpec.js
@@ -53,13 +53,6 @@ describe('DomUtil', function() {
 		});
 	});
 
-	describe('#documentIsLtr', function () {
-		it('returns true if doc direction is ltr', function () {
-			expect(L.DomUtil.documentIsLtr()).to.eql(true);
-			expect(L.DomUtil.documentIsLtr()).to.eql(true); // cached
-		});
-	});
-
 	describe('#getViewportOffset', function () {
 		it('calculates the viewport offset of an element', function () {
 			var div = document.createElement('div');

diff --git a/src/core/Browser.js b/src/core/Browser.js
@@ -16,6 +16,7 @@
 	    phantomjs = ua.indexOf('phantom') !== -1,
 	    android = ua.indexOf('android') !== -1,
 	    android23 = ua.search('android [23]') !== -1,
+		gecko = ua.indexOf('gecko') !== -1,
 
 	    mobile = typeof orientation !== undefined + '',
 	    msPointer = window.navigator && window.navigator.msPointerEnabled &&
@@ -72,6 +73,7 @@
 		ie7: ie7,
 		ielt9: ielt9,
 		webkit: webkit,
+		gecko: gecko && !webkit && !window.opera && !ie,
 
 		android: android,
 		android23: android23,

diff --git a/src/dom/DomEvent.js b/src/dom/DomEvent.js
@@ -143,11 +143,14 @@ L.DomEvent = {
 	},
 
 	getMousePosition: function (e, container) {
-
-		var ie7 = L.Browser.ie7,
-		    body = document.body,
+		var body = document.body,
 		    docEl = document.documentElement,
-		    x = e.pageX ? e.pageX - body.scrollLeft - docEl.scrollLeft: e.clientX,
+		    //gecko makes scrollLeft more negative as you scroll in rtl, other browsers don't
+			//ref: https://code.google.com/p/closure-library/source/browse/closure/goog/style/bidi.js
+			x = L.DomUtil.documentIsLtr() ?
+					(e.pageX ? e.pageX - body.scrollLeft - docEl.scrollLeft : e.clientX) :
+						(L.Browser.gecko ? e.pageX - body.scrollLeft - docEl.scrollLeft :
+						 e.pageX ? e.pageX - body.scrollLeft + docEl.scrollLeft : e.clientX),
 		    y = e.pageY ? e.pageY - body.scrollTop - docEl.scrollTop: e.clientY,
 		    pos = new L.Point(x, y);
 
@@ -159,19 +162,6 @@ L.DomEvent = {
 		    left = rect.left - container.clientLeft,
 		    top = rect.top - container.clientTop;
 
-		// webkit (and ie <= 7) handles RTL scrollLeft different to everyone else
-		// https://code.google.com/p/closure-library/source/browse/trunk/closure/goog/style/bidi.js
-		if (!L.DomUtil.documentIsLtr() && (L.Browser.webkit || ie7)) {
-			left += container.scrollWidth - container.clientWidth;
-
-			// ie7 shows the scrollbar by default and provides clientWidth counting it, so we
-			// need to add it back in if it is visible; scrollbar is on the left as we are RTL
-			if (ie7 && L.DomUtil.getStyle(container, 'overflow-y') !== 'hidden' &&
-			           L.DomUtil.getStyle(container, 'overflow') !== 'hidden') {
-				left += 17;
-			}
-		}
-
 		return pos._subtract(new L.Point(left, top));
 	},
 

diff --git a/src/dom/DomUtil.js b/src/dom/DomUtil.js
@@ -30,8 +30,7 @@ L.DomUtil = {
 		    el = element,
 		    docBody = document.body,
 		    docEl = document.documentElement,
-		    pos,
-		    ie7 = L.Browser.ie7;
+		    pos;
 
 		do {
 			top  += el.offsetTop  || 0;
@@ -78,19 +77,6 @@ L.DomUtil = {
 			top  -= el.scrollTop  || 0;
 			left -= el.scrollLeft || 0;
 
-			// webkit (and ie <= 7) handles RTL scrollLeft different to everyone else
-			// https://code.google.com/p/closure-library/source/browse/trunk/closure/goog/style/bidi.js
-			if (!L.DomUtil.documentIsLtr() && (L.Browser.webkit || ie7)) {
-				left += el.scrollWidth - el.clientWidth;
-
-				// ie7 shows the scrollbar by default and provides clientWidth counting it, so we
-				// need to add it back in if it is visible; scrollbar is on the left as we are RTL
-				if (ie7 && L.DomUtil.getStyle(el, 'overflow-y') !== 'hidden' &&
-				           L.DomUtil.getStyle(el, 'overflow') !== 'hidden') {
-					left += 17;
-				}
-			}
-
 			el = el.parentNode;
 		} while (el);