From e126af53a7f56285ee9700c0c2a8d5103c9ff783 Mon Sep 17 00:00:00 2001 From: Simon Lydell Date: Mon, 19 Sep 2016 19:07:43 +0200 Subject: [PATCH] Improve zoom handling for hint markers --- extension/lib/markable-elements.coffee | 6 +-- extension/lib/marker-container.coffee | 26 ++++++----- extension/lib/marker.coffee | 61 +++++++++++++------------- 3 files changed, 47 insertions(+), 46 deletions(-) diff --git a/extension/lib/markable-elements.coffee b/extension/lib/markable-elements.coffee index 43b4b52..d8d9d15 100644 --- a/extension/lib/markable-elements.coffee +++ b/extension/lib/markable-elements.coffee @@ -176,14 +176,14 @@ getElementShape = (elementData, tryRight, rects = null) -> offset = null if rects.length == 1 lefts = boxQuads - .map(({bounds}) -> bounds.left) - .filter((left) -> left >= nonCoveredPointRect.left) + .map(({bounds}) -> Math.floor(bounds.left)) + .filter((left) -> left >= Math.floor(nonCoveredPointRect.left)) offset = if lefts.length == 0 then null else Math.min(lefts...) else {bounds: {left}} = boxQuads[Math.min(nonCoveredPointRect.index, boxQuads.length - 1)] offset = Math.max(nonCoveredPointRect.left, left) - result.textOffset = offset - nonCoveredPointRect.left if offset? + result.textOffset = Math.floor(offset - nonCoveredPointRect.left) if offset? return result diff --git a/extension/lib/marker-container.coffee b/extension/lib/marker-container.coffee index cf8ba5d..101e31d 100644 --- a/extension/lib/marker-container.coffee +++ b/extension/lib/marker-container.coffee @@ -117,12 +117,23 @@ class MarkerContainer # only accessible in frame scripts) in `wrappers`, and insert them into # `@window`. injectHints: (wrappers, viewport, pass) -> - isComplementary = (pass == 'complementary') markers = Array(wrappers.length) markerMap = {} + zoom = 1 + if @adjustZoom + {ZoomManager, gBrowser: {selectedBrowser: browser}} = @window + # If “full zoom” is not used, it means that “Zoom text only” is enabled. + # If so, that “zoom” does not need to be taken into account. + # `.getCurrentMode()` is added by the “Default FullZoom Level” extension. + if ZoomManager.getCurrentMode?(browser) ? ZoomManager.useFullZoom + zoom = ZoomManager.getZoomForBrowser(browser) + for wrapper, index in wrappers - marker = new Marker(wrapper, @window.document, {isComplementary}) + marker = new Marker({ + wrapper, document: @window.document, viewport, zoom + isComplementary: (pass == 'complementary') + }) markers[index] = marker markerMap[wrapper.elementIndex] = marker if marker.isComplementary != @isComplementary or @enteredHint != '' @@ -181,22 +192,13 @@ class MarkerContainer @markHighlightedMarkers() - zoom = 1 - if @adjustZoom - {ZoomManager, gBrowser: {selectedBrowser: browser}} = @window - # If “full zoom” is not used, it means that “Zoom text only” is enabled. - # If so, that “zoom” does not need to be taken into account. - # `.getCurrentMode()` is added by the “Default FullZoom Level” extension. - if ZoomManager.getCurrentMode?(browser) ? ZoomManager.useFullZoom - zoom = ZoomManager.getZoomForBrowser(browser) - fragment = @window.document.createDocumentFragment() fragment.appendChild(marker.markerElement) for marker in markers @container.appendChild(fragment) # Must be done after the hints have been inserted into the DOM (see # `Marker::setPosition`). - marker.setPosition(viewport, zoom) for marker in markers + marker.setPosition() for marker in markers @markers.push(markers...) Object.assign(@markerMap, markerMap) diff --git a/extension/lib/marker.coffee b/extension/lib/marker.coffee index d452685..cd48a91 100644 --- a/extension/lib/marker.coffee +++ b/extension/lib/marker.coffee @@ -26,22 +26,20 @@ utils = require('./utils') class Marker # `@wrapper` is a stand-in for the element that the marker represents. See # `MarkerContainer::injectHints` for more information. - constructor: (@wrapper, @document, {@isComplementary}) -> + constructor: (options) -> + { + @wrapper, @document, @viewport, @zoom = 1, @isComplementary = false + } = options @elementShape = @wrapper.shape @markerElement = utils.createBox(@document, 'marker') @markerElement.setAttribute('data-type', @wrapper.type) @weight = @wrapper.combinedArea - @width = 0 - @height = 0 @hint = '' @originalHint = null @text = @wrapper.text?.toLowerCase() ? '' @visible = true @highlighted = false - @zoom = 1 - @viewport = null @position = null - @originalPosition = null @dx = 0 @dy = 0 @@ -58,7 +56,7 @@ class Marker # To be called when the marker has been both assigned a hint and inserted # into the DOM, and thus gotten a width and height. - setPosition: (@viewport, @zoom) -> + setPosition: -> { textOffset width: elementWidth @@ -67,15 +65,21 @@ class Marker rect = @markerElement.getBoundingClientRect() - @width = rect.width / @zoom - @height = rect.height / @zoom + # Take movements into account. + left += @dx + top += @dy - # Center the marker vertically on the non-covered point. - top -= Math.ceil(@height / 2) + # Make the position relative to the top frame. + left += offset.left + top += offset.top + + # Take the current zoom into account. + left *= @zoom + top *= @zoom if textOffset? # Move the marker just to the left of the text of its element. - left -= Math.max(0, @width - textOffset) + left -= Math.max(0, rect.width - textOffset * @zoom) else # Otherwise make sure that it doesn’t flow outside the right side of its # element. This is to avoid the following situation (where `+` is a small @@ -87,25 +91,20 @@ class Marker # bottom +Link text +Link text # middle DAG DAG # top E E - left -= Math.max(0, @width - elementWidth) + left -= Math.max(0, rect.width - elementWidth * @zoom) - # Make the position relative to the top frame. - left += offset.left - top += offset.top - - @originalPosition = {left, top} - @moveTo(left + @dx, top + @dy) + # Center the marker vertically on the non-covered point. + top -= Math.ceil(rect.height / 2) - moveTo: (left, top) -> # Make sure that the marker stays within the viewport. - left = Math.min(left, @viewport.right - @width) - top = Math.min(top, @viewport.bottom - @height) - left = Math.max(left, @viewport.left) - top = Math.max(top, @viewport.top) + left = Math.min(left, @zoom * @viewport.right - rect.width) + top = Math.min(top, @zoom * @viewport.bottom - rect.height) + left = Math.max(left, @zoom * @viewport.left) + top = Math.max(top, @zoom * @viewport.top) - # Take the current zoom into account. - left = Math.round(left * @zoom) - top = Math.round(top * @zoom) + # Use whole numbers for more deterministic positioning. + left = Math.round(left) + top = Math.round(top) # The positioning is absolute. @markerElement.style.left = "#{left}px" @@ -113,15 +112,15 @@ class Marker # For quick access. @position = { - left, right: left + @width, - top, bottom: top + @height, + left, right: Math.round(left + rect.width) + top, bottom: Math.round(top + rect.height) } updatePosition: (@dx, @dy) -> - @moveTo(@originalPosition.left + @dx, @originalPosition.top + @dy) + @setPosition() refreshPosition: -> - @setPosition(@viewport, @zoom) + @setPosition() setHint: (@hint) -> @originalHint ?= @hint -- 2.39.3