diff options
author | a1.gomes@sisa.samsung.com <a1.gomes@sisa.samsung.com@0039d316-1c4b-4281-b951-d872f2087c98> | 2013-11-15 20:33:46 +0000 |
---|---|---|
committer | a1.gomes@sisa.samsung.com <a1.gomes@sisa.samsung.com@0039d316-1c4b-4281-b951-d872f2087c98> | 2013-11-15 20:33:46 +0000 |
commit | 2aebb5e3d09df60d61f6f21110324d50b9bc2b7a (patch) | |
tree | 7f8ef7c6791132661aca5fcb99aa13db7667bb46 | |
parent | c427a06a820eafa0562dbb430b954e1e2d16f45f (diff) | |
download | chromium_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.grd | 2 | ||||
-rw-r--r-- | chrome/browser/resources/about_memory.js | 14 | ||||
-rw-r--r-- | chrome/browser/resources/extensions/extension_list.js | 7 | ||||
-rw-r--r-- | chrome/browser/resources/help/help_base_page.js | 7 | ||||
-rw-r--r-- | chrome/browser/resources/ntp_android/new_tab.html | 1 | ||||
-rw-r--r-- | chrome/browser/resources/ntp_android/ntp_android.js | 2 | ||||
-rw-r--r-- | chrome/browser/resources/options/options_page.js | 8 | ||||
-rw-r--r-- | chrome/browser/resources/uber/uber_utils.js | 7 | ||||
-rw-r--r-- | ui/webui/resources/js/util.js | 40 |
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; +} |