summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authora1.gomes@sisa.samsung.com <a1.gomes@sisa.samsung.com@0039d316-1c4b-4281-b951-d872f2087c98>2013-11-15 20:33:46 +0000
committera1.gomes@sisa.samsung.com <a1.gomes@sisa.samsung.com@0039d316-1c4b-4281-b951-d872f2087c98>2013-11-15 20:33:46 +0000
commit2aebb5e3d09df60d61f6f21110324d50b9bc2b7a (patch)
tree7f8ef7c6791132661aca5fcb99aa13db7667bb46
parentc427a06a820eafa0562dbb430b954e1e2d16f45f (diff)
downloadchromium_src-2aebb5e3d09df60d61f6f21110324d50b9bc2b7a.zip
chromium_src-2aebb5e3d09df60d61f6f21110324d50b9bc2b7a.tar.gz
chromium_src-2aebb5e3d09df60d61f6f21110324d50b9bc2b7a.tar.bz2
Make chrome/ be documentElement/body agnostic with regards to scrollTop/Left
Blink is in the process of getting itself in pair with how other Web engines' (Trident,Gecko,Presto) behavior with regards to scrollTop and scrollLeft when called for documentElement and body. This process has been proved itself to be of great impact within Chrome, given that its UI is written in HTML/JS/CSS. See for example bugs 305800, 304753, 305745, 304816, 305742, 305092. This CL tries to prevent Chrome from future similar breakages, as the Blink process of adapting to this change is not yet 100% finalized. It will also allow CLs 69143002 and 51553002 to land without breaking Chrome. CL also fixes driven-by issues like the usage of getElementById instead of $, so CQ can land the CL. Additionally, a PRESUBMIT hook is being considered as well, in order to advice programmers to not user {documentElement.body}.scroll{Top,Left} directly and instead use the helper added to resources/js/util.js BUG=316722 R=dbeam@chromium.org,newt@chromium.org,sky@chromium.org Review URL: https://codereview.chromium.org/68723003 git-svn-id: svn://svn.chromium.org/chrome/trunk/src@235414 0039d316-1c4b-4281-b951-d872f2087c98
-rw-r--r--chrome/browser/browser_resources.grd2
-rw-r--r--chrome/browser/resources/about_memory.js14
-rw-r--r--chrome/browser/resources/extensions/extension_list.js7
-rw-r--r--chrome/browser/resources/help/help_base_page.js7
-rw-r--r--chrome/browser/resources/ntp_android/new_tab.html1
-rw-r--r--chrome/browser/resources/ntp_android/ntp_android.js2
-rw-r--r--chrome/browser/resources/options/options_page.js8
-rw-r--r--chrome/browser/resources/uber/uber_utils.js7
-rw-r--r--ui/webui/resources/js/util.js40
9 files changed, 68 insertions, 20 deletions
diff --git a/chrome/browser/browser_resources.grd b/chrome/browser/browser_resources.grd
index dc563213..070bf5a 100644
--- a/chrome/browser/browser_resources.grd
+++ b/chrome/browser/browser_resources.grd
@@ -85,7 +85,7 @@
</if>
<include name="IDR_ABOUT_FLASH_HTML" file="resources\about_flash.html" flattenhtml="true" allowexternalscript="true" type="BINDATA" />
<include name="IDR_ABOUT_FLASH_JS" file="resources\about_flash.js" type="BINDATA" />
- <include name="IDR_ABOUT_MEMORY_JS" file="resources\about_memory.js" type="BINDATA" />
+ <include name="IDR_ABOUT_MEMORY_JS" file="resources\about_memory.js" flattenhtml="true" type="BINDATA" />
<include name="IDR_ABOUT_NACL_HTML" file="resources\about_nacl.html" flattenhtml="true" allowexternalscript="true" type="BINDATA" />
<include name="IDR_ABOUT_NACL_CSS" file="resources\about_nacl.css" flattenhtml="true" type="chrome_html" />
<include name="IDR_ABOUT_NACL_JS" file="resources\about_nacl.js" type="BINDATA" />
diff --git a/chrome/browser/resources/about_memory.js b/chrome/browser/resources/about_memory.js
index e3a6871..5626857 100644
--- a/chrome/browser/resources/about_memory.js
+++ b/chrome/browser/resources/about_memory.js
@@ -2,8 +2,10 @@
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.
+<include src="../../../ui/webui/resources/js/util.js"></include>
+
function reload() {
- if (document.getElementById('helpTooltip'))
+ if ($('helpTooltip'))
return;
history.go(0);
}
@@ -42,12 +44,14 @@ function handleHelpTooltipMouseOver(event) {
var width = el.offsetWidth;
var height = el.offsetHeight;
- if (event.pageX - width - 50 + document.documentElement.scrollLeft >= 0)
+ var scrollLeft = scrollLeftForDocument(document);
+ if (event.pageX - width - 50 + scrollLeft >= 0)
el.style.left = (event.pageX - width - 20) + 'px';
else
el.style.left = (event.pageX + 20) + 'px';
- if (event.pageY - height - 50 + document.documentElement.scrollTop >= 0)
+ var scrollTop = scrollTopForDocument(document);
+ if (event.pageY - height - 50 + scrollTop >= 0)
el.style.top = (event.pageY - height - 20) + 'px';
else
el.style.top = (event.pageY + 20) + 'px';
@@ -56,7 +60,7 @@ function handleHelpTooltipMouseOver(event) {
}
function handleHelpTooltipMouseOut(event) {
- var el = document.getElementById('helpTooltip');
+ var el = $('helpTooltip');
el.parentNode.removeChild(el);
}
@@ -72,7 +76,7 @@ function enableHelpTooltips() {
document.addEventListener('DOMContentLoaded', function() {
// This is the javascript code that processes the template:
var input = new JsEvalContext(loadTimeData.getValue('jstemplateData'));
- var output = document.getElementById('t');
+ var output = $('t');
jstProcess(input, output);
enableHelpTooltips();
diff --git a/chrome/browser/resources/extensions/extension_list.js b/chrome/browser/resources/extensions/extension_list.js
index 7fb517f..dbdf94b 100644
--- a/chrome/browser/resources/extensions/extension_list.js
+++ b/chrome/browser/resources/extensions/extension_list.js
@@ -59,8 +59,9 @@ cr.define('options', function() {
// the way at the top. That way it is clear that there are more elements
// above the element being scrolled to.
var scrollFudge = 1.2;
- document.documentElement.scrollTop = $(idToHighlight).offsetTop -
- scrollFudge * $(idToHighlight).clientHeight;
+ var scrollTop = $(idToHighlight).offsetTop - scrollFudge *
+ $(idToHighlight).clientHeight;
+ setScrollTopForDocument(document, scrollTop);
}
if (this.data_.extensions.length == 0)
@@ -345,7 +346,7 @@ cr.define('options', function() {
var pad = parseInt(getComputedStyle(node, null).marginTop, 10);
if (!isNaN(pad))
topScroll -= pad / 2;
- document.documentElement.scrollTop = topScroll;
+ setScrollTopForDocument(document, topScroll);
}
},
};
diff --git a/chrome/browser/resources/help/help_base_page.js b/chrome/browser/resources/help/help_base_page.js
index f07a9f1..15c3224 100644
--- a/chrome/browser/resources/help/help_base_page.js
+++ b/chrome/browser/resources/help/help_base_page.js
@@ -115,13 +115,14 @@ cr.define('help', function() {
* @private
*/
freeze_: function(freeze) {
+ var scrollLeft = scrollLeftForDocument(document);
if (freeze) {
- this.lastScrollTop = document.documentElement.scrollTop;
+ this.lastScrollTop = scrollTopForDocument(document);
document.body.style.overflow = 'hidden';
- window.scroll(document.documentElement.scrollLeft, 0);
+ window.scroll(scrollLeft, 0);
} else {
document.body.style.overflow = 'auto';
- window.scroll(document.documentElement.scrollLeft, this.lastScrollTop);
+ window.scroll(scrollLeft, this.lastScrollTop);
}
},
diff --git a/chrome/browser/resources/ntp_android/new_tab.html b/chrome/browser/resources/ntp_android/new_tab.html
index 8e29aa3..0a56508 100644
--- a/chrome/browser/resources/ntp_android/new_tab.html
+++ b/chrome/browser/resources/ntp_android/new_tab.html
@@ -72,6 +72,7 @@
</div>
</body>
<script src="../../../../ui/webui/resources/js/i18n_template.js"></script>
+ <script src="../../../../ui/webui/resources/js/util.js"></script>
<script>
i18nTemplate.process(document, templateData);
</script>
diff --git a/chrome/browser/resources/ntp_android/ntp_android.js b/chrome/browser/resources/ntp_android/ntp_android.js
index ad62da0..313884e 100644
--- a/chrome/browser/resources/ntp_android/ntp_android.js
+++ b/chrome/browser/resources/ntp_android/ntp_android.js
@@ -2153,7 +2153,7 @@ cr.define('ntp', function() {
currentPane = pane;
currentPaneIndex = paneIndex;
- document.documentElement.scrollTop = 0;
+ setScrollTopForDocument(document, 0);
var panelPrefix = sectionPrefixes[paneIndex];
var title = templateData[panelPrefix + '_document_title'];
diff --git a/chrome/browser/resources/options/options_page.js b/chrome/browser/resources/options/options_page.js
index e837c25..befb7c8 100644
--- a/chrome/browser/resources/options/options_page.js
+++ b/chrome/browser/resources/options/options_page.js
@@ -204,7 +204,7 @@ cr.define('options', function() {
var container = $('page-container');
var scrollTop = container.oldScrollTop || 0;
container.oldScrollTop = undefined;
- window.scroll(document.documentElement.scrollLeft, scrollTop);
+ window.scroll(scrollLeftForDocument(document), scrollTop);
};
/**
@@ -611,7 +611,7 @@ cr.define('options', function() {
if (freeze) {
// Lock the width, since auto width computation may change.
container.style.width = window.getComputedStyle(container).width;
- container.oldScrollTop = document.documentElement.scrollTop;
+ container.oldScrollTop = scrollTopForDocument(document);
container.classList.add('frozen');
var verticalPosition =
container.getBoundingClientRect().top - container.oldScrollTop;
@@ -703,8 +703,8 @@ cr.define('options', function() {
if (isRTL()) {
e.style.right = OptionsPage.horizontalOffset + 'px';
} else {
- e.style.left = OptionsPage.horizontalOffset -
- document.documentElement.scrollLeft + 'px';
+ var scrollLeft = scrollLeftForDocument(document);
+ e.style.left = OptionsPage.horizontalOffset - scrollLeft + 'px';
}
};
diff --git a/chrome/browser/resources/uber/uber_utils.js b/chrome/browser/resources/uber/uber_utils.js
index 04f8ba1..5f5c6d2 100644
--- a/chrome/browser/resources/uber/uber_utils.js
+++ b/chrome/browser/resources/uber/uber_utils.js
@@ -39,7 +39,8 @@ cr.define('uber', function() {
* @private
*/
function handleScroll() {
- var offset = document.documentElement.scrollLeft * -1;
+ var scrollLeft = scrollLeftForDocument(document);
+ var offset = scrollLeft * -1;
for (var i = 0; i < headerElements.length; i++) {
// As a workaround for http://crbug.com/231830, set the transform to
// 'none' rather than 0px.
@@ -47,7 +48,7 @@ cr.define('uber', function() {
'translateX(' + offset + 'px)' : 'none';
}
- invokeMethodOnParent('adjustToScroll', document.documentElement.scrollLeft);
+ invokeMethodOnParent('adjustToScroll', scrollLeft);
};
/**
@@ -67,7 +68,7 @@ cr.define('uber', function() {
* @private
*/
function handleFrameSelected() {
- document.documentElement.scrollLeft = 0;
+ setScrollTopForDocument(document, 0);
}
/**
diff --git a/ui/webui/resources/js/util.js b/ui/webui/resources/js/util.js
index 79f8d85..1fc1364 100644
--- a/ui/webui/resources/js/util.js
+++ b/ui/webui/resources/js/util.js
@@ -321,3 +321,43 @@ function ensureTransitionEndEvent(el, timeOut) {
cr.dispatchSimpleEvent(el, 'webkitTransitionEnd');
}, timeOut);
}
+
+/**
+ * Alias for document.scrollTop getter.
+ * @param {!HTMLDocument} doc The document node where information will be
+ * queried from.
+ * @return {number} The Y document scroll offset.
+ */
+function scrollTopForDocument(doc) {
+ return doc.documentElement.scrollTop || doc.body.scrollTop;
+}
+
+/**
+ * Alias for document.scrollTop setter.
+ * @param {!HTMLDocument} doc The document node where information will be
+ * queried from.
+ * @param {number} value The target Y scroll offset.
+ */
+function setScrollTopForDocument(doc, value) {
+ doc.documentElement.scrollTop = doc.body.scrollTop = value;
+}
+
+/**
+ * Alias for document.scrollLeft getter.
+ * @param {!HTMLDocument} doc The document node where information will be
+ * queried from.
+ * @return {number} The X document scroll offset.
+ */
+function scrollLeftForDocument(doc) {
+ return doc.documentElement.scrollLeft || doc.body.scrollLeft;
+}
+
+/**
+ * Alias for document.scrollLeft setter.
+ * @param {!HTMLDocument} doc The document node where information will be
+ * queried from.
+ * @param {number} value The target X scroll offset.
+ */
+function setScrollLeftForDocument(doc, value) {
+ doc.documentElement.scrollLeft = doc.body.scrollLeft = value;
+}