From 390a5c7045432c98999ae8bf9259fb9ae03bcb19 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Jacob=20M=C3=BCller?= <jacob.mueller.elz@gmail.com>
Date: Sun, 13 Sep 2020 21:38:32 +0200
Subject: [PATCH] fix(v-b-toggle): prevent scroll anchoring behavior (closes
 #5715) (#5769)

* fix(v-b-toggle): prevent scroll anchoring behavior

* Update form-textarea.js

* Update dom.js

* Update dom.spec.js

* Update dom.js

* Update modal-manager.js

* Update bv-collapse.js
---
 src/components/collapse/README.md             |  2 +-
 src/components/form-textarea/form-textarea.js |  8 ++--
 src/components/modal/helpers/modal-manager.js | 41 ++++++++++---------
 src/directives/toggle/toggle.js               | 18 ++++++--
 src/utils/bv-collapse.js                      | 20 ++++-----
 src/utils/dom.js                              | 24 +++++++++--
 src/utils/dom.spec.js                         | 34 +++++++++++----
 7 files changed, 98 insertions(+), 49 deletions(-)

diff --git a/src/components/collapse/README.md b/src/components/collapse/README.md
index e737984e9d6..e0649b78cc0 100644
--- a/src/components/collapse/README.md
+++ b/src/components/collapse/README.md
@@ -143,7 +143,7 @@ at a time.
 
 ```html
 <template>
-  <div role="tablist">
+  <div class="accordion" role="tablist">
     <b-card no-body class="mb-1">
       <b-card-header header-tag="header" class="p-1" role="tab">
         <b-button block v-b-toggle.accordion-1 variant="info">Accordion 1</b-button>
diff --git a/src/components/form-textarea/form-textarea.js b/src/components/form-textarea/form-textarea.js
index 15ae4b2e166..1b7fb148754 100644
--- a/src/components/form-textarea/form-textarea.js
+++ b/src/components/form-textarea/form-textarea.js
@@ -1,5 +1,5 @@
 import Vue from '../../utils/vue'
-import { getCS, isVisible, requestAF } from '../../utils/dom'
+import { getCS, getStyle, isVisible, requestAF, setStyle } from '../../utils/dom'
 import { isNull } from '../../utils/inspect'
 import { mathCeil, mathMax, mathMin } from '../../utils/math'
 import { toInteger, toFloat } from '../../utils/number'
@@ -171,13 +171,13 @@ export const BFormTextarea = /*#__PURE__*/ Vue.extend({
       const minHeight = lineHeight * this.computedMinRows + offset
 
       // Get the current style height (with `px` units)
-      const oldHeight = el.style.height || computedStyle.height
+      const oldHeight = getStyle(el, 'height') || computedStyle.height
       // Probe scrollHeight by temporarily changing the height to `auto`
-      el.style.height = 'auto'
+      setStyle(el, 'auto')
       const scrollHeight = el.scrollHeight
       // Place the original old height back on the element, just in case `computedProp`
       // returns the same value as before
-      el.style.height = oldHeight
+      setStyle(el, oldHeight)
 
       // Calculate content height in 'rows' (scrollHeight includes padding but not border)
       const contentRows = mathMax((scrollHeight - padding) / lineHeight, 2)
diff --git a/src/components/modal/helpers/modal-manager.js b/src/components/modal/helpers/modal-manager.js
index 474e89942dc..3c3a3c4d325 100644
--- a/src/components/modal/helpers/modal-manager.js
+++ b/src/components/modal/helpers/modal-manager.js
@@ -5,16 +5,18 @@
 
 import Vue from '../../../utils/vue'
 import {
+  addClass,
   getAttr,
+  getBCR,
+  getCS,
+  getStyle,
   hasAttr,
   removeAttr,
-  setAttr,
-  addClass,
   removeClass,
-  getBCR,
-  getCS,
+  requestAF,
   selectAll,
-  requestAF
+  setAttr,
+  setStyle
 } from '../../../utils/dom'
 import { isBrowser } from '../../../utils/env'
 import { isNull } from '../../../utils/inspect'
@@ -101,8 +103,9 @@ const ModalManager = /*#__PURE__*/ Vue.extend({
       if (isNull(this.baseZIndex) && isBrowser) {
         // Create a temporary `div.modal-backdrop` to get computed z-index
         const div = document.createElement('div')
-        div.className = 'modal-backdrop d-none'
-        div.style.display = 'none'
+        addClass(div, 'modal-backdrop')
+        addClass(div, 'd-none')
+        setStyle(div, 'display', 'none')
         document.body.appendChild(div)
         this.baseZIndex = toInteger(getCS(div).zIndex, DEFAULT_ZINDEX)
         document.body.removeChild(div)
@@ -113,7 +116,7 @@ const ModalManager = /*#__PURE__*/ Vue.extend({
       if (isNull(this.scrollbarWidth) && isBrowser) {
         // Create a temporary `div.measure-scrollbar` to get computed z-index
         const div = document.createElement('div')
-        div.className = 'modal-scrollbar-measure'
+        addClass(div, 'modal-scrollbar-measure')
         document.body.appendChild(div)
         this.scrollbarWidth = getBCR(div).width - div.clientWidth
         document.body.removeChild(div)
@@ -156,31 +159,31 @@ const ModalManager = /*#__PURE__*/ Vue.extend({
         // Adjust fixed content padding
         /* istanbul ignore next: difficult to test in JSDOM */
         selectAll(Selector.FIXED_CONTENT).forEach(el => {
-          const actualPadding = el.style.paddingRight
+          const actualPadding = getStyle(el, 'paddingRight')
           setAttr(el, 'data-padding-right', actualPadding)
-          el.style.paddingRight = `${toFloat(getCS(el).paddingRight, 0) + scrollbarWidth}px`
+          setStyle(el, 'paddingRight', `${toFloat(getCS(el).paddingRight, 0) + scrollbarWidth}px`)
           body._paddingChangedForModal.push(el)
         })
         // Adjust sticky content margin
         /* istanbul ignore next: difficult to test in JSDOM */
         selectAll(Selector.STICKY_CONTENT).forEach(el => /* istanbul ignore next */ {
-          const actualMargin = el.style.marginRight
+          const actualMargin = getStyle(el, 'marginRight')
           setAttr(el, 'data-margin-right', actualMargin)
-          el.style.marginRight = `${toFloat(getCS(el).marginRight, 0) - scrollbarWidth}px`
+          setStyle(el, 'marginRight', `${toFloat(getCS(el).marginRight, 0) - scrollbarWidth}px`)
           body._marginChangedForModal.push(el)
         })
         // Adjust <b-navbar-toggler> margin
         /* istanbul ignore next: difficult to test in JSDOM */
         selectAll(Selector.NAVBAR_TOGGLER).forEach(el => /* istanbul ignore next */ {
-          const actualMargin = el.style.marginRight
+          const actualMargin = getStyle(el, 'marginRight')
           setAttr(el, 'data-margin-right', actualMargin)
-          el.style.marginRight = `${toFloat(getCS(el).marginRight, 0) + scrollbarWidth}px`
+          setStyle(el, 'marginRight', `${toFloat(getCS(el).marginRight, 0) + scrollbarWidth}px`)
           body._marginChangedForModal.push(el)
         })
         // Adjust body padding
-        const actualPadding = body.style.paddingRight
+        const actualPadding = getStyle(body, 'paddingRight')
         setAttr(body, 'data-padding-right', actualPadding)
-        body.style.paddingRight = `${toFloat(getCS(body).paddingRight, 0) + scrollbarWidth}px`
+        setStyle(body, 'paddingRight', `${toFloat(getCS(body).paddingRight, 0) + scrollbarWidth}px`)
       }
     },
     resetScrollbar() {
@@ -190,7 +193,7 @@ const ModalManager = /*#__PURE__*/ Vue.extend({
         body._paddingChangedForModal.forEach(el => {
           /* istanbul ignore next: difficult to test in JSDOM */
           if (hasAttr(el, 'data-padding-right')) {
-            el.style.paddingRight = getAttr(el, 'data-padding-right') || ''
+            setStyle(el, 'paddingRight', getAttr(el, 'data-padding-right') || '')
             removeAttr(el, 'data-padding-right')
           }
         })
@@ -200,7 +203,7 @@ const ModalManager = /*#__PURE__*/ Vue.extend({
         body._marginChangedForModal.forEach(el => {
           /* istanbul ignore next: difficult to test in JSDOM */
           if (hasAttr(el, 'data-margin-right')) {
-            el.style.marginRight = getAttr(el, 'data-margin-right') || ''
+            setStyle(el, 'marginRight', getAttr(el, 'data-margin-right') || '')
             removeAttr(el, 'data-margin-right')
           }
         })
@@ -209,7 +212,7 @@ const ModalManager = /*#__PURE__*/ Vue.extend({
       body._marginChangedForModal = null
       // Restore body padding
       if (hasAttr(body, 'data-padding-right')) {
-        body.style.paddingRight = getAttr(body, 'data-padding-right') || ''
+        setStyle(body, 'paddingRight', getAttr(body, 'data-padding-right') || '')
         removeAttr(body, 'data-padding-right')
       }
     }
diff --git a/src/directives/toggle/toggle.js b/src/directives/toggle/toggle.js
index bb4c61ab083..8b0e0bc2115 100644
--- a/src/directives/toggle/toggle.js
+++ b/src/directives/toggle/toggle.js
@@ -9,8 +9,10 @@ import {
   isTag,
   removeAttr,
   removeClass,
+  removeStyle,
   requestAF,
-  setAttr
+  setAttr,
+  setStyle
 } from '../../utils/dom'
 import { isBrowser } from '../../utils/env'
 import { EVENT_OPTIONS_PASSIVE, eventOn, eventOff } from '../../utils/events'
@@ -46,6 +48,9 @@ const ATTR_ARIA_EXPANDED = 'aria-expanded'
 const ATTR_ROLE = 'role'
 const ATTR_TABINDEX = 'tabindex'
 
+// Commonly used style properties
+const STYLE_OVERFLOW_ANCHOR = 'overflow-anchor'
+
 // Emitted control event for collapse (emitted to collapse)
 export const EVENT_TOGGLE = 'bv::toggle::collapse'
 
@@ -194,13 +199,17 @@ const handleUpdate = (el, binding, vnode) => {
   // Parse list of target IDs
   const targets = getTargets(binding, el)
 
-  /* istanbul ignore else */
   // Ensure the `aria-controls` hasn't been overwritten
   // or removed when vnode updates
-  if (targets.length) {
+  // Also ensure to set `overflow-anchor` to `none` to prevent
+  // the browser's scroll anchoring behavior
+  /* istanbul ignore else */
+  if (targets.length > 0) {
     setAttr(el, ATTR_ARIA_CONTROLS, targets.join(' '))
+    setStyle(el, STYLE_OVERFLOW_ANCHOR, 'none')
   } else {
     removeAttr(el, ATTR_ARIA_CONTROLS)
+    removeStyle(el, STYLE_OVERFLOW_ANCHOR)
   }
 
   // Add/Update our click listener(s)
@@ -248,11 +257,12 @@ export const VBToggle = {
     resetProp(el, BV_TOGGLE_CLICK_HANDLER)
     resetProp(el, BV_TOGGLE_STATE)
     resetProp(el, BV_TOGGLE_TARGETS)
-    // Reset classes/attrs
+    // Reset classes/attrs/styles
     removeClass(el, CLASS_BV_TOGGLE_COLLAPSED)
     removeClass(el, CLASS_BV_TOGGLE_NOT_COLLAPSED)
     removeAttr(el, ATTR_ARIA_EXPANDED)
     removeAttr(el, ATTR_ARIA_CONTROLS)
     removeAttr(el, ATTR_ROLE)
+    removeStyle(el, STYLE_OVERFLOW_ANCHOR)
   }
 }
diff --git a/src/utils/bv-collapse.js b/src/utils/bv-collapse.js
index 16f15c1ba8e..3756626eaf8 100644
--- a/src/utils/bv-collapse.js
+++ b/src/utils/bv-collapse.js
@@ -7,32 +7,32 @@
 //   in-place after the transition completes
 import Vue from './vue'
 import { mergeData } from 'vue-functional-data-merge'
-import { getBCR, reflow, requestAF } from './dom'
+import { getBCR, reflow, removeStyle, requestAF, setStyle } from './dom'
 
 // Transition event handler helpers
 const onEnter = el => {
-  el.style.height = 0
-  // Animaton frame delay needed for `appear` to work
+  setStyle(el, 'height', 0)
+  // In a `requestAF()` for `appear` to work
   requestAF(() => {
     reflow(el)
-    el.style.height = `${el.scrollHeight}px`
+    setStyle(el, 'height', `${el.scrollHeight}px`)
   })
 }
 
 const onAfterEnter = el => {
-  el.style.height = null
+  removeStyle(el, 'height')
 }
 
 const onLeave = el => {
-  el.style.height = 'auto'
-  el.style.display = 'block'
-  el.style.height = `${getBCR(el).height}px`
+  setStyle(el, 'height', 'auto')
+  setStyle(el, 'display', 'block')
+  setStyle(el, 'height', `${getBCR(el).height}px`)
   reflow(el)
-  el.style.height = 0
+  setStyle(el, 'height', 0)
 }
 
 const onAfterLeave = el => {
-  el.style.height = null
+  removeStyle(el, 'height')
 }
 
 // Default transition props
diff --git a/src/utils/dom.js b/src/utils/dom.js
index b1857122606..cd69ca6ad7e 100644
--- a/src/utils/dom.js
+++ b/src/utils/dom.js
@@ -88,7 +88,7 @@ export const isVisible = el => {
     // are not a direct descendant of document.body
     return false
   }
-  if (el.style.display === 'none') {
+  if (getStyle(el, 'display') === 'none') {
     // We do this check to help with vue-test-utils when using v-show
     /* istanbul ignore next */
     return false
@@ -174,9 +174,9 @@ export const hasClass = (el, className) => {
 }
 
 // Set an attribute on an element
-export const setAttr = (el, attr, val) => {
+export const setAttr = (el, attr, value) => {
   if (attr && isElement(el)) {
-    el.setAttribute(attr, val)
+    el.setAttribute(attr, value)
   }
 }
 
@@ -195,6 +195,24 @@ export const getAttr = (el, attr) => (attr && isElement(el) ? el.getAttribute(at
 // Returns `true` or `false`, or `null` if element not found
 export const hasAttr = (el, attr) => (attr && isElement(el) ? el.hasAttribute(attr) : null)
 
+// Set an style property on an element
+export const setStyle = (el, prop, value) => {
+  if (prop && isElement(el)) {
+    el.style[prop] = value
+  }
+}
+
+// Remove an style property from an element
+export const removeStyle = (el, prop) => {
+  if (prop && isElement(el)) {
+    el.style[prop] = ''
+  }
+}
+
+// Get an style property value from an element
+// Returns `null` if not found
+export const getStyle = (el, prop) => (prop && isElement(el) ? el.style[prop] || null : null)
+
 // Return the Bounding Client Rect of an element
 // Returns `null` if not an element
 /* istanbul ignore next: getBoundingClientRect() doesn't work in JSDOM */
diff --git a/src/utils/dom.spec.js b/src/utils/dom.spec.js
index 34397d6ec97..1c43133c152 100644
--- a/src/utils/dom.spec.js
+++ b/src/utils/dom.spec.js
@@ -1,22 +1,23 @@
 import { mount } from '@vue/test-utils'
 import { createContainer } from '../../tests/utils'
 import {
-  isElement,
-  isDisabled,
-  contains,
   closest,
+  contains,
+  getAttr,
+  getStyle,
+  hasAttr,
+  hasClass,
+  isDisabled,
+  isElement,
   matches,
   select,
-  selectAll,
-  hasAttr,
-  getAttr,
-  hasClass
+  selectAll
 } from './dom'
 
 const template = `
 <div id="a" class="foo">
   <div class="bar">
-    <span class="barspan foobar"></span>
+    <span class="barspan foobar" style="color: red;"></span>
   </div>
   <div class="baz">
     <button id="button1" aria-label="label">btn 1</button>
@@ -197,6 +198,23 @@ describe('utils/dom', () => {
     wrapper.destroy()
   })
 
+  it('getStyle() works', async () => {
+    const wrapper = mount(App, {
+      attachTo: createContainer()
+    })
+
+    expect(wrapper).toBeDefined()
+
+    const $span = wrapper.find('span.barspan')
+    expect($span).toBeDefined()
+    expect($span.exists()).toBe(true)
+    expect(getStyle($span.element, 'color')).toBe('red')
+    expect(getStyle($span.element, 'width')).toBe(null)
+    expect(getStyle(null, 'color')).toBe(null)
+
+    wrapper.destroy()
+  })
+
   it('select() works', async () => {
     const wrapper = mount(App, {
       attachTo: createContainer()