Skip to content

Commit

Permalink
Complete guarding (fixes #127)
Browse files Browse the repository at this point in the history
* Remove magic numbers in all setTimeout() calls.
* If landmarks might be out-of-date, re-scan when using the button or
  keyboard shortcuts.
* Make injected landmarks test page less verbose.
  • Loading branch information
matatk committed Jan 18, 2018
1 parent 368596b commit e339db9
Show file tree
Hide file tree
Showing 4 changed files with 51 additions and 54 deletions.
3 changes: 2 additions & 1 deletion src/static/content.focusing.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
/* exported ElementFocuser */

function ElementFocuser() {
const momentaryBorderTime = 2000
let currentlySelectedElement


Expand Down Expand Up @@ -38,7 +39,7 @@ function ElementFocuser() {
addBorder(element)

if (borderTypePref === 'momentary') {
setTimeout(() => removeBorder(element), 2000)
setTimeout(() => removeBorder(element), momentaryBorderTime)
}
}

Expand Down
15 changes: 15 additions & 0 deletions src/static/content.overall.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
'use strict'
/* global LandmarksFinder ElementFocuser PauseHandler */
const outOfDateTime = 2000
const logger = new Logger()
let observer = null

Expand Down Expand Up @@ -61,22 +62,27 @@ function messageHandler(message, sender, sendResponse) {
switch (message.request) {
case 'get-landmarks':
// The pop-up is requesting the list of landmarks on the page
handleOutdatedResults()
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)
handleOutdatedResults()
checkFocusElement(() => lf.getLandmarkElement(message.index))
break
case 'next-landmark':
// Triggered by keyboard shortcut
handleOutdatedResults()
checkFocusElement(lf.getNextLandmarkElement)
break
case 'prev-landmark':
// Triggered by keyboard shortcut
handleOutdatedResults()
checkFocusElement(lf.getPreviousLandmarkElement)
break
case 'main-landmark': {
handleOutdatedResults()
const mainElement = lf.getMainElement()
if (mainElement) {
ef.focusElement(mainElement)
Expand All @@ -102,6 +108,13 @@ function messageHandler(message, sender, sendResponse) {
}
}

function handleOutdatedResults() {
if (ph.getPauseTime() > outOfDateTime) {
logger.log(`Landmarks may be out of date (pause: ${ph.getPauseTime()}); scanning now...`)
findLandmarksAndUpdateBadge()
}
}

function checkFocusElement(callbackReturningElement) {
if (lf.getNumberOfLandmarks() === 0) {
alert(browser.i18n.getMessage('noLandmarksFound') + '.')
Expand Down Expand Up @@ -132,6 +145,7 @@ function sendUpdateBadgeMessage() {
// been retired because the extension was unloaded/reloaded. In which
// case, we don't want to keep handling mutations.
if (observer) {
logger.log('Disconnecting observer from retired content script')
observer.disconnect()
} else {
throw error
Expand Down Expand Up @@ -161,6 +175,7 @@ function setUpMutationObserver() {
ph.run(
function() {
if (shouldRefreshLandmarkss(mutations)) {
logger.log('SCAN mutation')
findLandmarksAndUpdateBadge()
}
},
Expand Down
5 changes: 4 additions & 1 deletion src/static/content.pausing.js
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,6 @@ function PauseHandler(logger) {
this.run = function(guardedTask, scheduledTask) {
const now = Date.now()
if (now > lastEvent + pause) {
logger.log('SCAN mutation')
guardedTask()
lastEvent = now
} else if (!haveIncreasedPauseAndScheduledTask) {
Expand All @@ -80,4 +79,8 @@ function PauseHandler(logger) {
haveIncreasedPauseAndScheduledTask = true
}
}

this.getPauseTime = function() {
return pause
}
}
82 changes: 30 additions & 52 deletions test/manual-test-injected-landmark.html
Original file line number Diff line number Diff line change
Expand Up @@ -9,14 +9,14 @@
<h1>Test injected landmark</h1>
</header>
<main>
<p>Main content.</p>
<button id="outer-injector">Inject a landmark</button>
<button id="inner-injector" disabled>
Inject a landmark inside the other
</button>
<button id="the-cleaner" disabled>
Remove all injected bits and bobs
</button>
<p>Main content.</p>
<button id="outer-injector">Inject a landmark</button>
<button id="inner-injector" disabled>
Inject a landmark inside the other
</button>
<button id="the-cleaner" disabled>
Remove all injected bits and bobs
</button>
</main>
<footer>
<p>This is the footer.</p>
Expand All @@ -30,70 +30,48 @@ <h1>Test injected landmark</h1>

function makeSection(level, heading, text) {
const section = document.createElement('section')
const sectionHeader = document.createElement(`h${level}`)
const sectionHeaderText = document.createTextNode(heading)
const sectionPara = document.createElement('p')
const sectionParaText = document.createTextNode(text)
const sectionHeader = document.createElement(`h${level}`)
const sectionHeaderText = document.createTextNode(heading)
const sectionPara = document.createElement('p')
const sectionParaText = document.createTextNode(text)

sectionHeader.appendChild(sectionHeaderText)
section.appendChild(sectionHeader)
sectionHeader.appendChild(sectionHeaderText)
section.appendChild(sectionHeader)

sectionPara.appendChild(sectionParaText)
section.appendChild(sectionPara)
sectionPara.appendChild(sectionParaText)
section.appendChild(sectionPara)

sectionHeader.id = `section-header-${idCounter++}`
section.setAttribute('aria-labelledby', sectionHeader.id)
sectionHeader.id = `section-header-${idCounter++}`
section.setAttribute('aria-labelledby', sectionHeader.id)

return section
return section
}

document.getElementById(btnOuterInjectorId).onclick = function() {
const section = makeSection(2, 'Outer injected section',
'A section nested within the main region.')
section.id = outerInjectedSectionId
document.getElementsByTagName('main')[0].appendChild(section)
'A section nested within the main region.')
section.id = outerInjectedSectionId
document.getElementsByTagName('main')[0].appendChild(section)

this.disabled = true
document.getElementById(btnInnerInjectorId).removeAttribute('disabled')
this.disabled = true
document.getElementById(btnInnerInjectorId).removeAttribute('disabled')
}

document.getElementById(btnInnerInjectorId).onclick = function() {
const innerSection = makeSection(3, 'Inner injected section',
'A further nested section.')
document.getElementById(outerInjectedSectionId).appendChild(innerSection)
'A further nested section.')
document.getElementById(outerInjectedSectionId).appendChild(innerSection)

this.disabled = true
document.getElementById(btnCleanerId).removeAttribute('disabled')
this.disabled = true
document.getElementById(btnCleanerId).removeAttribute('disabled')
}

document.getElementById(btnCleanerId).onclick = function() {
document.getElementById(outerInjectedSectionId).remove()

this.disabled = true
document.getElementById(btnOuterInjectorId).removeAttribute('disabled')
this.disabled = true
document.getElementById(btnOuterInjectorId).removeAttribute('disabled')
}

const observer = new MutationObserver(function(mutations) {
mutations.forEach(function(mutation) {
console.log(mutation)
if (mutation.type === 'childList') {
mutation.addedNodes.forEach(function(addedNode) {
if (addedNode.nodeType === Node.ELEMENT_NODE) {
console.log('Added node:', addedNode)
}
})
mutation.removedNodes.forEach(function(removedNode) {
console.log('Removed node:', removedNode)
})
}
})
})

observer.observe(document, {
attributes: true,
childList: true,
subtree: true
})
</script>
</body>
</html>

0 comments on commit e339db9

Please sign in to comment.