From 7203c03073234515049bc0ddeebd832d543cec1c Mon Sep 17 00:00:00 2001 From: Simon Lydell Date: Tue, 19 Apr 2016 20:16:40 +0200 Subject: [PATCH] Move viewport calculations from hints.coffee to utils.coffee It's cleaner and more reusable. This also fixes a slight bug for markers in frames. Previously, if the frame had padding and/or borders its calculated viewport was a tiny bit too large. Now, padding and borders are taken into account. --- extension/lib/commands-frame.coffee | 5 ++- extension/lib/commands.coffee | 13 ++++---- extension/lib/hints.coffee | 52 ++++++----------------------- extension/lib/utils.coffee | 49 +++++++++++++++++++++++++++ 4 files changed, 71 insertions(+), 48 deletions(-) diff --git a/extension/lib/commands-frame.coffee b/extension/lib/commands-frame.coffee index 628b321..9d62821 100644 --- a/extension/lib/commands-frame.coffee +++ b/extension/lib/commands-frame.coffee @@ -153,7 +153,10 @@ helper_follow = ({id, combine = true}, matcher, args) -> return wrapper - return hints.getMarkableElementsAndViewport(vim.content, filter) + return { + viewport: utils.getWindowViewport(vim.content) + wrappers: hints.getMarkableElements(vim.content, filter) + } commands.follow = helper_follow.bind(null, {id: 'normal'}, ({vim, element, getElementShape}) -> diff --git a/extension/lib/commands.coffee b/extension/lib/commands.coffee index 6067342..a664382 100644 --- a/extension/lib/commands.coffee +++ b/extension/lib/commands.coffee @@ -556,24 +556,25 @@ commands.click_browser_element = ({vim}) -> utils.focusElement(element) utils.simulateMouseEvents(element, sequence) - createMarkers = ({wrappers, viewport}) -> + createMarkers = (wrappers) -> + viewport = utils.getWindowViewport(vim.window) {markers} = hints.injectHints(vim.window, wrappers, viewport, { hint_chars: vim.options.hint_chars ui: true }) return markers - result = hints.getMarkableElementsAndViewport( + wrappers = hints.getMarkableElements( vim.window, filter.bind(null, {markEverything: false}) ) - if result.wrappers.length > 0 - storage = vim.enterMode('hints', createMarkers(result), callback) + if wrappers.length > 0 + storage = vim.enterMode('hints', createMarkers(wrappers), callback) storage.markEverything = -> - newResult = hints.getMarkableElementsAndViewport( + newWrappers = hints.getMarkableElements( vim.window, filter.bind(null, {markEverything: true}) ) - storage.markers = createMarkers(newResult) + storage.markers = createMarkers(newWrappers) else vim.notify(translate('notification.follow.none')) diff --git a/extension/lib/hints.coffee b/extension/lib/hints.coffee index 0d58bbc..fa6248b 100644 --- a/extension/lib/hints.coffee +++ b/extension/lib/hints.coffee @@ -142,11 +142,11 @@ injectHints = (window, wrappers, viewport, options) -> return {markers, markerMap} -getMarkableElementsAndViewport = (window, filter) -> +getMarkableElements = (window, filter) -> viewport = utils.getWindowViewport(window) wrappers = [] - getMarkableElements(window, viewport, wrappers, filter) - return {wrappers, viewport} + _getMarkableElements(window, viewport, wrappers, filter) + return wrappers # `filter` is a function that is given every element in every frame of the page. # It should return wrapper objects for markable elements and a falsy value for @@ -155,7 +155,7 @@ getMarkableElementsAndViewport = (window, filter) -> # each frame. It might sound expensive to go through _every_ element, but that’s # actually what other methods like using XPath or CSS selectors would need to do # anyway behind the scenes. -getMarkableElements = (window, viewport, wrappers, filter, parents = []) -> +_getMarkableElements = (window, viewport, wrappers, filter, parents = []) -> {document} = window for element in getAllElements(document) when element instanceof Element @@ -172,33 +172,11 @@ getMarkableElements = (window, viewport, wrappers, filter, parents = []) -> wrappers.push(wrapper) for frame in window.frames when frame.frameElement - rect = frame.frameElement.getBoundingClientRect() # Frames only have one. - continue unless isInsideViewport(rect, viewport) - - # Calculate the visible part of the frame, according to the parent. - # coffeelint: disable=colon_assignment_spacing - {clientWidth, clientHeight} = frame.document.documentElement - frameViewport = { - left: Math.max(viewport.left - rect.left, 0) - top: Math.max(viewport.top - rect.top, 0) - right: clientWidth + Math.min(viewport.right - rect.right, 0) - bottom: clientHeight + Math.min(viewport.bottom - rect.bottom, 0) - } - # coffeelint: enable=colon_assignment_spacing - - # `.getComputedStyle()` may return `null` if the computed style isn’t - # availble yet. If so, consider the element not visible. - continue unless computedStyle = window.getComputedStyle(frame.frameElement) - offset = { - left: rect.left + - parseFloat(computedStyle.getPropertyValue('border-left-width')) + - parseFloat(computedStyle.getPropertyValue('padding-left')) - top: rect.top + - parseFloat(computedStyle.getPropertyValue('border-top-width')) + - parseFloat(computedStyle.getPropertyValue('padding-top')) - } - - getMarkableElements( + continue unless result = utils.getFrameViewport( + frame.frameElement, viewport + ) + {viewport: frameViewport, offset} = result + _getMarkableElements( frame, frameViewport, wrappers, filter, parents.concat({window, offset}) ) @@ -234,7 +212,7 @@ getRects = (element, viewport) -> # rectangle for each line, since each line may be of different length, for # example. That allows us to properly add hints to line-wrapped links. return Array.filter( - element.getClientRects(), (rect) -> isInsideViewport(rect, viewport) + element.getClientRects(), (rect) -> utils.isInsideViewport(rect, viewport) ) # Returns the “shape” of `element`: @@ -285,14 +263,6 @@ getElementShape = (window, viewport, parents, element, rects = null) -> nonCoveredPoint, area: totalArea } -MINIMUM_EDGE_DISTANCE = 4 -isInsideViewport = (rect, viewport) -> - return \ - rect.left <= viewport.right - MINIMUM_EDGE_DISTANCE and - rect.top <= viewport.bottom + MINIMUM_EDGE_DISTANCE and - rect.right >= viewport.left + MINIMUM_EDGE_DISTANCE and - rect.bottom >= viewport.top - MINIMUM_EDGE_DISTANCE - adjustRectToViewport = (rect, viewport) -> # The right and bottom values are subtracted by 1 because # `document.elementFromPoint(right, bottom)` does not return the element @@ -438,5 +408,5 @@ contains = (element, elementAtPoint) -> module.exports = { removeHints injectHints - getMarkableElementsAndViewport + getMarkableElements } diff --git a/extension/lib/utils.coffee b/extension/lib/utils.coffee index 646429c..a417f34 100644 --- a/extension/lib/utils.coffee +++ b/extension/lib/utils.coffee @@ -51,6 +51,8 @@ EVENTS_CLICK_XUL = ['click', 'command'] EVENTS_HOVER_START = ['mouseover', 'mouseenter', 'mousemove'] EVENTS_HOVER_END = ['mouseout', 'mouseleave'] +MINIMUM_EDGE_DISTANCE = 4 + # Element classification helpers @@ -335,6 +337,44 @@ createBox = (document, className = '', parent = null, text = null) -> parent.appendChild(box) if parent? return box +getFrameViewport = (frame, parentViewport) -> + rect = frame.getBoundingClientRect() + return null unless isInsideViewport(rect, parentViewport) + + # `.getComputedStyle()` may return `null` if the computed style isn’t availble + # yet. If so, consider the element not visible. + return null unless computedStyle = frame.ownerGlobal.getComputedStyle(frame) + offset = { + left: rect.left + + parseFloat(computedStyle.getPropertyValue('border-left-width')) + + parseFloat(computedStyle.getPropertyValue('padding-left')) + top: rect.top + + parseFloat(computedStyle.getPropertyValue('border-top-width')) + + parseFloat(computedStyle.getPropertyValue('padding-top')) + right: rect.right - + parseFloat(computedStyle.getPropertyValue('border-right-width')) - + parseFloat(computedStyle.getPropertyValue('padding-right')) + bottom: rect.bottom - + parseFloat(computedStyle.getPropertyValue('border-bottom-width')) - + parseFloat(computedStyle.getPropertyValue('padding-bottom')) + } + + # Calculate the visible part of the frame, according to the parent. + viewport = getWindowViewport(frame.contentWindow) + left = viewport.left + Math.max(parentViewport.left - offset.left, 0) + top = viewport.top + Math.max(parentViewport.top - offset.top, 0) + right = viewport.right + Math.min(parentViewport.right - offset.right, 0) + bottom = viewport.bottom + Math.min(parentViewport.bottom - offset.bottom, 0) + + return { + viewport: { + left, top, right, bottom + width: right - left + height: bottom - top + } + offset + } + # In quirks mode (when the page lacks a doctype), such as on Hackernews, # `` is considered the root element rather than ``. getRootElement = (document) -> @@ -417,6 +457,13 @@ insertText = (input, value) -> isDetached = (element) -> return not element.ownerDocument?.documentElement?.contains?(element) +isInsideViewport = (rect, viewport) -> + return \ + rect.left <= viewport.right - MINIMUM_EDGE_DISTANCE and + rect.top <= viewport.bottom + MINIMUM_EDGE_DISTANCE and + rect.right >= viewport.left + MINIMUM_EDGE_DISTANCE and + rect.bottom >= viewport.top - MINIMUM_EDGE_DISTANCE + isPositionFixed = (element) -> computedStyle = element.ownerGlobal.getComputedStyle(element) return computedStyle?.getPropertyValue('position') == 'fixed' @@ -605,11 +652,13 @@ module.exports = { containsDeep createBox getRootElement + getFrameViewport getViewportCappedClientHeight getWindowViewport injectTemporaryPopup insertText isDetached + isInsideViewport isPositionFixed querySelectorAllDeep scroll -- 2.39.3