From 515f0a4bb9153019d556ef6ffd9cdd0e6453530b Mon Sep 17 00:00:00 2001 From: Simon Lydell Date: Tue, 30 Jul 2013 14:47:26 +0200 Subject: [PATCH] Group all hint injection code in hints.coffee Before, the hint/marker injection code was spread out in both hints.coffee and marker.coffee. Creation and injection of markers does not really belong to the Marker class. It is just a wrapper around a single markable element. Now, all hint/marker creation and injection is done in hints.coffee. This has several benefits: The code is a lot easier to understand, and the number of loops over arrays of markers could be reduced a lot, enhancing performance. I've also cleaned the code up, as well as removed code that was totally useless: Before, `element.getClientRects()` was scanned for rectangles matching the `isRectOk()` test. However, none of them could ever pass, since `isRectOk()` looks for width and height properties on the rectangles, which do not even exist (however, they do on the rectangle returned by `element.getBoundingClientRect()`). This also fixes the uncaught ReferenceErrors complaining about undefined width and height properties. This also removed unnecessary loops, which enhances performance even more. Some of these changes could have been made outside the huffman branch, I realize now. However, it would take some work, and I'm not sure it is worth it. An improvement I noticed with this patch: It is now possible to click the green, hidden until hover install buttons on AMO: https://addons.mozilla.org/firefox/addon/VimFx/versions/ --- extension/packages/hints.coffee | 162 ++++++++++++++++++++++++++----- extension/packages/marker.coffee | 127 +----------------------- 2 files changed, 141 insertions(+), 148 deletions(-) diff --git a/extension/packages/hints.coffee b/extension/packages/hints.coffee index aedcbbf..cbb3418 100644 --- a/extension/packages/hints.coffee +++ b/extension/packages/hints.coffee @@ -1,4 +1,5 @@ utils = require 'utils' +{ getPref } = require 'prefs' { Marker } = require 'marker' { addHuffmanCodeWordsTo } = require 'huffman' @@ -6,18 +7,51 @@ utils = require 'utils' HTMLDocument = Ci.nsIDOMHTMLDocument XULDocument = Ci.nsIDOMXULDocument +XPathResult = Ci.nsIDOMXPathResult CONTAINER_ID = 'VimFxHintMarkerContainer' -createHintsContainer = (document) -> - container = document.createElement('div') - container.id = CONTAINER_ID - container.className = 'VimFxReset' - return container +# All the following elements qualify for their own marker in hints mode +MARKABLE_ELEMENTS = [ + "a" + "iframe" + "area[@href]" + "textarea" + "button" + "select" + "input[not(@type='hidden' or @disabled or @readonly)]" + "embed" + "object" +] + +# All elements that have one or more of the following properties +# qualify for their own marker in hints mode +MARKABLE_ELEMENT_PROPERTIES = [ + "@tabindex" + "@onclick" + "@onmousedown" + "@onmouseup" + "@oncommand" + "@role='link'" + "@role='button'" + "contains(@class, 'button')" + "contains(@class, 'js-new-tweets-bar')" + "@contenteditable='' or translate(@contenteditable, 'TRUE', 'true')='true'" +] -# Creates and injects hint markers into the DOM + +# Remove previously injected hints from the DOM +removeHints = (document) -> + if container = document.getElementById(CONTAINER_ID) + document.documentElement.removeChild(container) + + for frame in document.defaultView.frames + removeHints(frame.document) + + +# Like `injectMarkers`, but also sets hints for the markers injectHints = (document) -> - markers = getMarkers(document) + markers = injectMarkers(document) hintChars = utils.getHintChars() addHuffmanCodeWordsTo markers, @@ -26,38 +60,122 @@ injectHints = (document) -> return markers -getMarkers = (document) -> + +# Creates and injects markers into the DOM +injectMarkers = (document) -> # First remove previous hints container removeHints(document) # For now we aren't able to handle hint markers in XUL Documents :( if document instanceof HTMLDocument# or document instanceof XULDocument if document.documentElement - # Find and create markers - markers = Marker.createMarkers(document) - - container = createHintsContainer(document) - # For performance use Document Fragment fragment = document.createDocumentFragment() - for marker in markers - fragment.appendChild(marker.markerElement) + # Select all markable elements in the document, create markers + # for each of them, and position them on the page. + # Note that the markers are not given hints. + set = getMarkableElements(document) + markers = [] + for i in [0...set.snapshotLength] by 1 + element = set.snapshotItem(i) + if rect = getElementRect(element) + marker = new Marker(element) + + marker.setPosition(rect) + fragment.appendChild(marker.markerElement) + + marker.weight = rect.area + + markers.push(marker) + + container = createHintsContainer(document) container.appendChild(fragment) document.documentElement.appendChild(container) for frame in document.defaultView.frames - markers = markers.concat(getMarkers(frame.document)) + markers = markers.concat(injectMarkers(frame.document)) return markers or [] -# Remove previously injected hints from the DOM -removeHints = (document) -> - if container = document.getElementById(CONTAINER_ID) - document.documentElement.removeChild(container) - for frame in document.defaultView.frames - removeHints(frame.document) +createHintsContainer = (document) -> + container = document.createElement('div') + container.id = CONTAINER_ID + container.className = 'VimFxReset' + return container + + +# Returns elements that qualify for hint markers in hints mode. +# Generates and memoizes an XPath query internally +getMarkableElements = do -> + # Some preparations done on startup + elements = [ + MARKABLE_ELEMENTS... + "*[#{ MARKABLE_ELEMENT_PROPERTIES.join(' or ') }]" + ] + + reduce = (m, rule) -> + m.concat(["//#{ rule }", "//xhtml:#{ rule }"]) + xpath = elements + .reduce(reduce, []) + .join(' | ') + + namespaceResolver = (namespace) -> + if namespace == 'xhtml' then 'http://www.w3.org/1999/xhtml' else null + + # The actual function that will return the desired elements + return (document, resultType = XPathResult.ORDERED_NODE_SNAPSHOT_TYPE) -> + return document.evaluate(xpath, document.documentElement, namespaceResolver, resultType, null) + + +# Uses `element.getBoundingClientRect()`. If that does not return a visible rectange, then looks at +# the children of the markable node. +# +# The logic has been copied over from Vimiun originally. +getElementRect = (element) -> + document = element.ownerDocument + window = document.defaultView + docElem = document.documentElement + body = document.body + + clientTop = docElem.clientTop or body?.clientTop or 0 + clientLeft = docElem.clientLeft or body?.clientLeft or 0 + scrollTop = window.pageYOffset or docElem.scrollTop + scrollLeft = window.pageXOffset or docElem.scrollLeft + + clientRect = element.getBoundingClientRect() + + if isRectOk(clientRect, window) + return { + top: clientRect.top + scrollTop - clientTop + left: clientRect.left + scrollLeft - clientLeft + width: clientRect.width + height: clientRect.height + area: clientRect.width * clientRect.height + } + + # If the rect has 0 dimensions, then check what's inside. + # Floated or absolutely positioned elements are of particular interest. + if clientRect.width is 0 or clientRect.height is 0 + for childElement in element.children + if computedStyle = window.getComputedStyle(childElement, null) + if computedStyle.getPropertyValue('float') != 'none' or \ + computedStyle.getPropertyValue('position') == 'absolute' + + return getElementRect(childElement) + + return + + +# Checks if the given TextRectangle object qualifies +# for its own Marker with respect to the `window` object +isRectOk = (rect, window) -> + minimum = 2 + rect.width > minimum and rect.height > minimum and \ + rect.top > -minimum and rect.left > -minimum and \ + rect.top < window.innerHeight - minimum and \ + rect.left < window.innerWidth - minimum exports.injectHints = injectHints diff --git a/extension/packages/marker.coffee b/extension/packages/marker.coffee index 46f5ea9..ea4ab56 100644 --- a/extension/packages/marker.coffee +++ b/extension/packages/marker.coffee @@ -1,38 +1,3 @@ -{ getPref } = require 'prefs' -utils = require 'utils' - -{ interfaces: Ci } = Components - -XPathResult = Ci.nsIDOMXPathResult - -# All elements that have one or more of the following properties -# qualify for their own marker in hints mode -MARKABLE_ELEMENT_PROPERTIES = [ - "@tabindex" - "@onclick" - "@onmousedown" - "@onmouseup" - "@oncommand" - "@role='link'" - "@role='button'" - "contains(@class, 'button')" - "contains(@class, 'js-new-tweets-bar')" - "@contenteditable='' or translate(@contenteditable, 'TRUE', 'true')='true'" -] - -# All the following elements qualify for their own marker in hints mode -MARKABLE_ELEMENTS = [ - "a" - "iframe" - "area[@href]" - "textarea" - "button" - "select" - "input[not(@type='hidden' or @disabled or @readonly)]" - "embed" - "object" -] - # Marker class wraps the markable element and provides # methods to manipulate the markers class Marker @@ -56,7 +21,7 @@ class Marker # Assigns hint string to the marker setHint: (@hintChars) -> - # number of hint chars that have been matched so far + # Hint chars that have been matched so far @enteredHintChars = '' document = @element.ownerDocument @@ -102,94 +67,4 @@ class Marker return @hintChars == @enteredHintChars -# Selects all markable elements on the page, creates markers -# for each of them. The markers are then positioned on the page. -# Note that the markers are not given any hints at this point. -# -# The array of markers is returned -Marker.createMarkers = (document) -> - set = getMarkableElements(document) - - markers = [] - for i in [0...set.snapshotLength] by 1 - element = set.snapshotItem(i) - if rect = getElementRect(element) - marker = new Marker(element) - marker.setPosition(rect) - marker.weight = rect.area - markers.push(marker) - - return markers - - -# Returns elements that qualify for hint markers in hints mode. -# Generates and memoizes an XPath query internally -getMarkableElements = do -> - # Some preparations done on startup - elements = Array.concat \ - MARKABLE_ELEMENTS, - ["*[#{ MARKABLE_ELEMENT_PROPERTIES.join(' or ') }]"] - - xpath = elements.reduce((m, rule) -> - m.concat(["//#{ rule }", "//xhtml:#{ rule }"]) - , []).join(' | ') - - namespaceResolver = (namespace) -> - if namespace == 'xhtml' then 'http://www.w3.org/1999/xhtml' else null - - # The actual function that will return the desired elements - return (document, resultType = XPathResult.ORDERED_NODE_SNAPSHOT_TYPE) -> - document.evaluate(xpath, document.documentElement, namespaceResolver, resultType, null) - -# Checks if the given TextRectangle object qualifies -# for its own Marker with respect to the `window` object -isRectOk = (rect, window) -> - rect.width > 2 and rect.height > 2 and \ - rect.top > -2 and rect.left > -2 and \ - rect.top < window.innerHeight - 2 and \ - rect.left < window.innerWidth - 2 - -# Will scan through `element.getClientRects()` and look for -# the first visible rectange. If there are no visible rectangles, then -# will look at the children of the markable node. -# -# The logic has been copied over from Vimiun -getElementRect = (element) -> - document = element.ownerDocument - window = document.defaultView - docElem = document.documentElement - body = document.body - - clientTop = docElem.clientTop || body?.clientTop || 0 - clientLeft = docElem.clientLeft || body?.clientLeft || 0 - scrollTop = window.pageYOffset || docElem.scrollTop - scrollLeft = window.pageXOffset || docElem.scrollLeft - - clientRect = element.getBoundingClientRect() - rects = [rect for rect in element.getClientRects()] - rects.push(clientRect) - - for rect in rects - if isRectOk(rect, window) - return { - top: rect.top + scrollTop - clientTop - left: rect.left + scrollLeft - clientLeft - width: rect.width - height: rect.height - area: clientRect.width * clientRect.height - } - - # If the element has 0 dimentions then check what's inside. - # Floated or absolutely positioned elements are of particular interest - for rect in rects - if rect.width == 0 or rect.height == 0 - for childElement in element.children - if computedStyle = window.getComputedStyle(childElement, null) - if computedStyle.getPropertyValue('float') != 'none' or \ - computedStyle.getPropertyValue('position') == 'absolute' - - return childRect if childRect = getElementRect(childElement) - - return undefined - exports.Marker = Marker -- 2.39.3