Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

perf: Streamlining scans and messaging; Debugging info #428

Merged
merged 39 commits into from
Jun 26, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
39 commits
Select commit Hold shift + click to select a range
f733878
Debug by tracking messages
matatk Jun 13, 2021
9e51e42
Add rollup-split plugin
matatk Jun 14, 2021
e224d87
Make the debugging info only appear in debug builds
matatk Jun 14, 2021
877fdd1
Make the de-duplicating content script start messages official
matatk Jun 13, 2021
b52d79d
Make the debugging prettier and add some FIXMEs
matatk Jun 15, 2021
c53a760
Fix devtools disconnect and background errors
matatk Jun 15, 2021
2abb3aa
Try not scanning for landmarks in visibility handler
matatk Jun 15, 2021
469cf28
Reflect page visibility on content script load
matatk Jun 16, 2021
195b3d6
Counting mutations; Scanning on startup
matatk Jun 16, 2021
962a978
Mutation counting refinement; Outdated check
matatk Jun 16, 2021
12c20c3
Improve clarity of UI message
matatk Jun 16, 2021
8fbd4da
Comments and Tweaks
matatk Jun 17, 2021
b8f5a33
Really clean up debugging messages
matatk Jun 17, 2021
8931318
More!
matatk Jun 18, 2021
f489baf
Clarify messages further; More visibility debugging
matatk Jun 18, 2021
b93b19d
Don't ask for DevTools state if starting hidden; message clearups
matatk Jun 18, 2021
e0177f7
Remove some debugging messages; Tweak TODO/FIXMEs
matatk Jun 18, 2021
1660976
Change scanning behaviour; Tidyups
matatk Jun 18, 2021
b438c70
Tidy up background script
matatk Jun 18, 2021
3c1ea0f
Tidy up _gui.js a bit
matatk Jun 18, 2021
b5976a5
Properly detect if we are paused or not
matatk Jun 18, 2021
8c4e385
Tweak content script comment after some thought
matatk Jun 18, 2021
020dcd7
Fix for non-debug code; Tweak comments
matatk Jun 19, 2021
98d7532
Better handling of startup and tab-switch
matatk Jun 24, 2021
599dd8a
Content script naming clarity and factoring
matatk Jun 24, 2021
b4ff6fe
Log instead of throwing errors
matatk Jun 24, 2021
56f0516
Send landmarks when (re-)observing
matatk Jun 24, 2021
cdd9f40
Fix logic of outdated landmark checking
matatk Jun 24, 2021
b8e8d65
Rename
matatk Jun 24, 2021
004f714
TODO: Investigate null landmarksFinder possibility
matatk Jun 24, 2021
f9e02b2
More useful error messages (hopefully)
matatk Jun 24, 2021
3f55684
Upgrade some notes
matatk Jun 25, 2021
2037952
Remove Chrome 91 workaround as bug was fixed
matatk Jun 25, 2021
9caeafb
Factor out DevTools-handling
matatk Jun 25, 2021
c2a2026
Harmonise throwing
matatk Jun 25, 2021
b87defb
Streamline scanner-changing
matatk Jun 26, 2021
119f2b2
Clear up comment
matatk Jun 26, 2021
321e941
Factor out dealing with current/all tabs
matatk Jun 26, 2021
817cc39
Tweak docs/output of compare script
matatk Jun 26, 2021
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
92 changes: 92 additions & 0 deletions package-lock.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@
]
},
"devDependencies": {
"@rollup/plugin-strip": "^2.0.1",
"addons-linter": "^3.7.0",
"archiver-promise": "~1.0",
"ava": "^3.15.0",
Expand Down
9 changes: 8 additions & 1 deletion scripts/build.js
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ const { minify } = require('terser')
const replace = require('replace-in-file')
const rollup = require('rollup')
const sharp = require('sharp')
const strip = require('@rollup/plugin-strip')
const terser = require('rollup-plugin-terser').terser


Expand Down Expand Up @@ -324,7 +325,13 @@ async function bundleCode(browser, debug) {

bundleOption.input = {
input: ioPair.mainSourceFile,
plugins: [terser(makeTerserOptions(defines)), esformatter()]
plugins: debug
? [ terser(makeTerserOptions(defines)), esformatter() ]
: [
strip({ functions: ['debugSend', 'debugLog'] }),
terser(makeTerserOptions(defines)),
esformatter()
]
}

bundleOption.output = {
Expand Down
5 changes: 3 additions & 2 deletions scripts/compare.sh
Original file line number Diff line number Diff line change
Expand Up @@ -17,8 +17,9 @@ echo " <browser> <browser> d"
echo " <browser> d"
echo
echo "You may want to run the following first"
echo " node scripts/build.js --browser all --debug"
echo " node scripts/build.js --browser all"
echo " node scripts/build.js [--skip-linting] --skip-zipping --browser all --debug"
echo " node scripts/build.js [--skip-linting] --skip-zipping --browser all"
echo

if [ ! "$1" ] || [ ! "$2" ]; then
echo "Missing first or second arg"
Expand Down
8 changes: 7 additions & 1 deletion src/assemble/_landmarksFinder.js
Original file line number Diff line number Diff line change
Expand Up @@ -105,6 +105,8 @@ export default function LandmarksFinder(win, doc) {
let currentlySelectedElement

function updateSelectedIndexAndReturnElementInfo(index) {
// TODO: Don't need an index check, as we trust the source. Does that
// mean we also don't need the length check?
if (landmarks.length === 0) return
currentlySelectedIndex = index
currentlySelectedElement = landmarks[index].element
Expand All @@ -113,7 +115,7 @@ export default function LandmarksFinder(win, doc) {
role: landmarks[index].role,
roleDescription: landmarks[index].roleDescription,
label: landmarks[index].label
// No need to send the selector this time
// No need to send the selector or warnings
}
}

Expand Down Expand Up @@ -189,6 +191,10 @@ export default function LandmarksFinder(win, doc) {
parentLandmark = element
}

// One just one page I've seen an error here in Chrome (91) which seems
// to be a bug, because only one HTMLElement was returned; not an
// HTMLCollection. Checking for this would cause a slowdown, so
// ignoring for now.
for (const elementChild of element.children) {
getLandmarks(elementChild, depth, parentLandmark)
}
Expand Down
71 changes: 41 additions & 30 deletions src/assemble/gui.html
Original file line number Diff line number Diff line change
Expand Up @@ -99,38 +99,49 @@
</details>
</dd>

<dt id="statsScansTerm" data-message="statsScansTerm"></dt></dt>
<dd>
<span id="scans">0</span>
<details class="tooltip">
<summary class="definition" aria-labelledby="define-statsScansTerm statsScansTerm">
<span id="define-statsScansTerm" data-message="statsDefine" class="visually-hidden"></span>
</summary>
<p data-message="statsScansDefinition"></p>
</details>
</dd>
<dt id="statsMutationScansTerm" data-message="statsMutationScansTerm"></dt>
<dd>
<span id="mutationScans">0</span>
<details class="tooltip">
<summary class="definition" aria-labelledby="define-statsMutationScansTerm statsMutationScansTerm">
<span id="define-statsMutationScansTerm" data-message="statsDefine" class="visually-hidden"></span>
</summary>
<p data-message="statsMutationScansDefinition"></p>
</details>
</dd>

<dt id="statsPauseTerm" data-message="statsPauseTerm"></dt>
<dd>
<span id="pause">&mdash;</span>ms
<details class="tooltip">
<summary class="definition" aria-labelledby="define-statsPauseTerm statsPauseTerm">
<span id="define-statsPauseTerm" data-message="statsDefine" class="visually-hidden"></span>
</summary>
<p data-message="statsPauseDefinition"></p>
</details>
</dd>
<dt id="statsNonMutationScansTerm" data-message="statsNonMutationScansTerm"></dt>
<dd>
<span id="nonMutationScans">0</span>
<details class="tooltip">
<summary class="definition" aria-labelledby="define-statsNonMutationScansTerm statsNonMutationScansTerm">
<span id="define-statsNonMutationScansTerm" data-message="statsDefine" class="visually-hidden"></span>
</summary>
<p data-message="statsNonMutationScansDefinition"></p>
</details>
</dd>

<dt id="statsDurationTerm" data-message="statsDurationTerm"></dt>
<dd>
<span id="duration">&mdash;</span>ms
<details class="tooltip">
<summary class="definition" aria-labelledby="define-statsDurationTerm statsDurationTerm">
<span id="define-statsDurationTerm" data-message="statsDefine" class="visually-hidden"></span>
</summary>
<p data-message="statsDurationDefinition"></p>
</details>
</dd>
<dt id="statsPauseTerm" data-message="statsPauseTerm"></dt>
<dd>
<span id="pause">&mdash;</span>ms
<details class="tooltip">
<summary class="definition" aria-labelledby="define-statsPauseTerm statsPauseTerm">
<span id="define-statsPauseTerm" data-message="statsDefine" class="visually-hidden"></span>
</summary>
<p data-message="statsPauseDefinition"></p>
</details>
</dd>

<dt id="statsDurationTerm" data-message="statsDurationTerm"></dt>
<dd>
<span id="duration">&mdash;</span>ms
<details class="tooltip">
<summary class="definition" aria-labelledby="define-statsDurationTerm statsDurationTerm">
<span id="define-statsDurationTerm" data-message="statsDefine" class="visually-hidden"></span>
</summary>
<p data-message="statsDurationDefinition"></p>
</details>
</dd>
</dl>
<p data-message="statsInfo"></p>
</details>
Expand Down
20 changes: 14 additions & 6 deletions src/assemble/messages.common.en_GB.json
Original file line number Diff line number Diff line change
Expand Up @@ -247,23 +247,31 @@
},

"statsCheckedMutationsDefinition": {
"message": "Mutations that Landmarks has tested to see if they are relevant."
"message": "Mutations that Landmarks has tested to see if they are relevant (mutations that happen during a pause in scanning are ignored)."
},

"statsScansTerm": {
"message": "Scans:"
"statsMutationScansTerm": {
"message": "Mutation scans:"
},

"statsScansDefinition": {
"message": "Number of times the page has been scanned for landmarks."
"statsMutationScansDefinition": {
"message": "Number of times the page has been scanned for landmarks due to mutations. Includes scans triggered directly by mutations, as well as scans scheduled for after a pause brought on by multiple successive mutations."
},

"statsNonMutationScansTerm": {
"message": "Non-mutation scans:"
},

"statsNonMutationScansDefinition": {
"message": "Number of times the page has been scanned for landmarks for other reasons, such as: page load; use of History API; user command whilst scanning is paused due to prior mutations."
},

"statsPauseTerm": {
"message": "Pause time:"
},

"statsPauseDefinition": {
"message": "Time during which subsequent changes to the page will be ignored."
"message": "Time window during which subsequent changes to the page will be ignored."
},

"statsDurationTerm": {
Expand Down
Loading