Skip to content

Commit

Permalink
Simplify content script bootstrapping (closes #137)
Browse files Browse the repository at this point in the history 
* Remove unused translation message.
* Simplify LandmarksFinder API and rename functions for clarity (they
  are getters). This fixes a bug that was there for ages whereby it was
  not calling the function to check the number of landmarks found (in
  checkFocusElement()).
* Vastly simplify the bootstrapping code in content.overall.js.
* Move Logger to content.overall.js from content.pausing.js and
  generalise.
* Remove Logger from content.pausing.js.
  • Loading branch information
@matatk
matatk committed Jan 12, 2018
1 parent abf8277 commit e19e1ee
Show file tree
Hide file tree
Showing 5 changed files with 100 additions and 126 deletions.
4 changes: 0 additions & 4 deletions src/static/_locales/en_GB/messages.json
View file
Original file line number Diff line number Diff line change
Expand Up @@ -49,10 +49,6 @@
"message": "No landmarks found"
},

"pageNotLoadedYet": {
"message": "The page has not yet been checked for landmarks; please try again"
},

"tryReloading": {
"message": "Try reloading the page"
},
Expand Down
23 changes: 6 additions & 17 deletions src/static/content.finder.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -63,8 +63,6 @@ function LandmarksFinder(win, doc) {
// label: (string or null) -- author-supplied label
// element: (HTML*Element) -- in-memory element

let haveSearchedForLandmarks = false


//
// Keeping track of landmark navigation
Expand Down Expand Up @@ -259,15 +257,6 @@ function LandmarksFinder(win, doc) {
mainElementIndex = -1
currentlySelectedIndex = -1
getLandmarks(doc.body.parentNode, 0) // supports role on <body>
haveSearchedForLandmarks = true
}

this.haveSearchedForLandmarks = function() {
return haveSearchedForLandmarks
}

this.reset = function() {
haveSearchedForLandmarks = false
}

this.filter = function() {
Expand All @@ -278,26 +267,26 @@ function LandmarksFinder(win, doc) {
}))
}

this.numberOfLandmarks = function() {
return haveSearchedForLandmarks ? landmarks.length : -1
this.getNumberOfLandmarks = function() {
return landmarks.length
}

this.nextLandmarkElement = function() {
this.getNextLandmarkElement = function() {
return updateSelectedIndexAndReturnElement(
(currentlySelectedIndex + 1) % landmarks.length)
}

this.previousLandmarkElement = function() {
this.getPreviousLandmarkElement = function() {
return updateSelectedIndexAndReturnElement(
(currentlySelectedIndex <= 0) ?
landmarks.length - 1 : currentlySelectedIndex - 1)
}

this.landmarkElement = function(index) {
this.getLandmarkElement = function(index) {
return updateSelectedIndexAndReturnElement(index)
}

this.selectMainElement = function() {
this.getMainElement = function() {
return mainElementIndex < 0 ?
null : updateSelectedIndexAndReturnElement(mainElementIndex)
}
Expand Down
153 changes: 87 additions & 66 deletions src/static/content.overall.js
View file
Original file line number Diff line number Diff line change
@@ -1,31 +1,54 @@
'use strict'
/* global LandmarksFinder ElementFocuser PauseHandler */
const logger = new Logger()
let observer = null

const lf = new LandmarksFinder(window, document)
const ef = new ElementFocuser()
const ph = new PauseHandler()
const ph = new PauseHandler(logger)


//
// Guard for focusing elements
// Log messages according to user setting
//

// Check that it is OK to focus an landmark element
function checkFocusElement(callbackReturningElement) {
// The user may use the keyboard commands before landmarks have been found
// However, the content script will run and find any landmarks very soon
// after the page has loaded.
if (!lf.haveSearchedForLandmarks()) {
alert(browser.i18n.getMessage('pageNotLoadedYet') + '.')
return
function Logger() {
let logging = null

function getDebugInfoOption(callback) {
browser.storage.sync.get({
debugInfo: false
}, function(items) {
logging = items.debugInfo
if (callback) {
callback()
}
})
}

if (lf.numberOfLandmarks === 0) {
alert(browser.i18n.getMessage('noLandmarksFound') + '.')
return
function handleOptionsChange(changes) {
const changedOptions = Object.keys(changes)
if (changedOptions.includes('debugInfo')) {
logging = changes.debugInfo.newValue
}
}

ef.focusElement(callbackReturningElement())
// We may wish to log messages right way, but the call to get the user
// setting is asynchronous. Therefore, we need to pass our bootstrapping
// code as a callback that is run when the option has been fetched.
//
// Also, we only define the log() function after successfully initing, so
// as to trap any errant uses of the logger.
this.init = function(callback) {
getDebugInfoOption(callback)
browser.storage.onChanged.addListener(handleOptionsChange)

this.log = function() {
if (logging) {
console.log.apply(null, arguments)
}
}
}
}


Expand All @@ -34,36 +57,27 @@ function checkFocusElement(callbackReturningElement) {
//

// Act on requests from the background or pop-up scripts
browser.runtime.onMessage.addListener(function(message, sender, sendResponse) {
function messageHandler(message, sender, sendResponse) {
switch (message.request) {
case 'get-landmarks':
// The pop-up is requesting the list of landmarks on the page

if (!lf.haveSearchedForLandmarks()) {
sendResponse('wait')
}
// We only guard for landmarks having been found here because the
// other messages still need to be handled regardless (or, in some
// cases, won't be recieved until after the pop-up has been
// displayed, so this check only needs to be here).

sendResponse(lf.filter())
break
case 'focus-landmark':
// Triggered by clicking on an item in the pop-up, or indirectly
// via one of the keyboard shortcuts (if landmarks are present)
checkFocusElement(() => lf.landmarkElement(message.index))
checkFocusElement(() => lf.getLandmarkElement(message.index))
break
case 'next-landmark':
// Triggered by keyboard shortcut
checkFocusElement(lf.nextLandmarkElement)
checkFocusElement(lf.getNextLandmarkElement)
break
case 'prev-landmark':
// Triggered by keyboard shortcut
checkFocusElement(lf.previousLandmarkElement)
checkFocusElement(lf.getPreviousLandmarkElement)
break
case 'main-landmark': {
const mainElement = lf.selectMainElement()
const mainElement = lf.getMainElement()
if (mainElement) {
ef.focusElement(mainElement)
} else {
Expand All @@ -78,68 +92,70 @@ browser.runtime.onMessage.addListener(function(message, sender, sendResponse) {
// (indicating that its content has changed substantially). When
// this happens, we should treat it as a new page, and fetch
// landmarks again when asked.
logger.log('Landmarks: trigger-refresh')
ef.removeBorderOnCurrentlySelectedElement()
bootstrap()
findLandmarksAndUpdateBadge()
break
default:
throw Error(
'Landmarks: content script received unknown message: '
throw Error('Landmarks: content script received unknown message: '
+ message.request)
}
})
}

function checkFocusElement(callbackReturningElement) {
if (lf.getNumberOfLandmarks() === 0) {
alert(browser.i18n.getMessage('noLandmarksFound') + '.')
return
}

ef.focusElement(callbackReturningElement())
}


//
// Actually finding landmarks
//

function findLandmarksAndUpdateBadge() {
lf.find()
sendUpdateBadgeMessage()
}

function sendUpdateBadgeMessage() {
// Let the background script know how many landmarks were found, so
// that it can update the browser action badge.
try {
browser.runtime.sendMessage({
request: 'update-badge',
landmarks: lf.numberOfLandmarks()
landmarks: lf.getNumberOfLandmarks()
})
} catch (error) {
throw Error('I found this:', error.message)
// The most likely error is that, on !Firefox this content script has
// been retired because the extension was unloaded/reloaded. In which
// case, we don't want to keep handling mutations.
if (observer) {
observer.disconnect()
} else {
throw error
}
}
}

function findLandmarksAndUpdateBadge() {
lf.find()
sendUpdateBadgeMessage()
}


//
// Bootstrapping and mutation observer setup
//

// Most pages I've tried have got to a readyState of 'complete' within
// 10-100ms. Therefore this should easily be sufficient.
function bootstrap() {
const attemptInterval = 500
const maximumAttempts = 4
let landmarkFindingAttempts = 0

lf.reset()
sendUpdateBadgeMessage()

function bootstrapCore() {
landmarkFindingAttempts += 1

if (document.readyState === 'complete') {
findLandmarksAndUpdateBadge()
setUpMutationObserver()
} else if (landmarkFindingAttempts < maximumAttempts) {
setTimeout(bootstrapCore, attemptInterval)
} else {
throw Error('Landmarks: page not available for scanning '
+ `after ${maximumAttempts} attempts.`)
}
}

setTimeout(bootstrapCore, attemptInterval)
logger.init(() => {
logger.log('Bootstrapping Landmarks')
logger.log(`Document state: ${document.readyState}`)
findLandmarksAndUpdateBadge()
setUpMutationObserver()
browser.runtime.onMessage.addListener(messageHandler)
})
}

function setUpMutationObserver() {
const observer = new MutationObserver((mutations) => {
observer = new MutationObserver((mutations) => {
// Guard against being innundated by mutation events
// (which happens in e.g. Google Docs)
ph.run(
Expand Down Expand Up @@ -195,4 +211,9 @@ function shouldRefreshLandmarkss(mutations) {
return false
}


//
// Entry point
//

bootstrap()
44 changes: 7 additions & 37 deletions src/static/content.pausing.js
View file
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
'use strict'
/* exported PauseHandler */

function PauseHandler() {
function PauseHandler(logger) {
//
// Constants
//
Expand All @@ -21,36 +21,6 @@ function PauseHandler() {
let lastEvent = Date.now()
let decreasePauseTimeout = null
let haveIncreasedPauseAndScheduledTask = false
let logging = false


//
// Reflecting options and Loggign
//

function log() {
if (logging) {
console.log.apply(null, arguments)
}
}

function getDebugInfoOption() {
browser.storage.sync.get({
debugInfo: false
}, function(items) {
logging = items.debugInfo
})
}

function handleOptionsChange(changes) {
const changedOptions = Object.keys(changes)
if (changedOptions.includes('debugInfo')) {
logging = changes.debugInfo.newValue
}
}

getDebugInfoOption()
browser.storage.onChanged.addListener(handleOptionsChange)


//
Expand All @@ -63,7 +33,7 @@ function PauseHandler() {
if (pause >= maxPause) {
pause = maxPause
}
log('Increased pause to:', pause)
logger.log('Increased pause to:', pause)
}

function decreasePause() {
Expand All @@ -78,13 +48,13 @@ function PauseHandler() {
} else {
decreasePause()
}
log('Decreased pause to:', pause)
logger.log('Decreased pause to:', pause)
}

function stopDecreasingPause() {
clearTimeout(decreasePauseTimeout)
decreasePauseTimeout = null
log('Stopped decreasing the pause')
logger.log('Stopped decreasing the pause')
}


Expand All @@ -95,14 +65,14 @@ function PauseHandler() {
this.run = function(guardedTask, scheduledTask) {
const now = Date.now()
if (now > lastEvent + pause) {
log('SCAN mutation')
logger.log('SCAN mutation')
guardedTask()
lastEvent = now
} else if (!haveIncreasedPauseAndScheduledTask) {
increasePause()
log('Scheduling scan in:', pause)
logger.log('Scheduling scan in:', pause)
setTimeout(() => {
log('SCAN as scheduled')
logger.log('SCAN as scheduled')
scheduledTask()
decreasePause()
haveIncreasedPauseAndScheduledTask = false
Expand Down
2 changes: 0 additions & 2 deletions src/static/popup.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -29,8 +29,6 @@ function handleLandmarksResponse(response) {
} else {
makeLandmarksTree(response, display)
}
} else if (response === 'wait') {
addText(display, browser.i18n.getMessage('pageNotLoadedYet'))
} else {
addText(display, errorString() + 'content script sent: ' + response)
}
Expand Down

0 comments on commit e19e1ee

Please sign in to comment.