feat: update to manifest v3 (Resolves: #32, #33, #29) #40

cindyli · 2022-12-08T17:45:36Z

Resolves: #32
Resolves: #33
Resolves: #29

Migration docs:

amb26 · 2022-12-17T16:56:23Z

package.json

+        "clean:build": "rimraf dist",
+        "clean:test": "rimraf reports coverage",
+        "build": "run-s -l clean:build build:*",
+        "build:dist": "cpy . ../dist --cwd=src; cpy . ../../dist/lib/infusion --cwd=node_modules/infusion"


I think the ; operator is non-portable - I get an error running this line on Windows, replacing it with & allows the build to complete

amb26 · 2022-12-17T16:59:33Z

src/js/enactors.js

+
+uioPlus.enactor.tableOfContents.injectToCContainer = function (that) {
+    if (!that.locate("tocContainer").length) {
+        if (that.content.length === 1) {


A comment explaining why the variant strategy is necessary here would be helpful

I'll try to explain, and hopefully we can come up with a suitable comment.

The ToC component from Infusion relies on the container for the ToC component to be located within the page. However, we cannot rely on the ToC's container being pre-added to the page with UIO+ as it runs on arbitrary sites. uioPlus.enactor.tableOfContents.injectToCContainer will inject the necessary ToC container markup into the pages content, and this container will then be used by the ToC component.

The first if guards against adding the ToC container if one already exists.

The second if/else will either append the ToC container to the content or to the enactor's container if the content isn't found. The content is located by means of uioPlus.contentView defined above. It provides a means for looking for the pages main content area (e.g. <main>, <article> etc.). In some cases, UIO+ is not able to locate the content area. For example a page marked up like <body><div></div><div></div></body>.

I've added @jobara's comment into the code.

All code review suggestions so far have been addressed.

jobara · 2022-12-20T14:18:57Z

package.json

        "nyc": "15.1.0",
        "rimraf": "3.0.2",
-        "sinon": "13.0.1",


I'm getting an error when trying to run the tests that it can't find sinon.

Interesting. sinon-chrome pulls in sinon on my computer. I've added the sinon. Can you try again? Thanks.

jobara · 2022-12-20T14:29:52Z

With UIO+ enabled the browser console reports the following error:

{
    "message": "Assertion failure - check console for more details: No container element was found for selector .uioPlus",
    "stack": "Error: Assertion failure - check console for more details: No container element was found for selector .uioPlus\n    at new fluid.FluidError (chrome-extension://gbenfaceipnpnobakclnelebaeinlelm/lib/infusion/dist/infusion-uio.js:128:2470)\n    at fluid.builtinFail (chrome-extension://gbenfaceipnpnobakclnelebaeinlelm/lib/infusion/dist/infusion-uio.js:128:3038)\n    at fluid.event.firer.fire (chrome-extension://gbenfaceipnpnobakclnelebaeinlelm/lib/infusion/dist/infusion-uio.js:128:25883)\n    at fluid.fail (chrome-extension://gbenfaceipnpnobakclnelebaeinlelm/lib/infusion/dist/infusion-uio.js:128:3289)\n    at fluid.container (chrome-extension://gbenfaceipnpnobakclnelebaeinlelm/lib/infusion/dist/infusion-uio.js:137:224673)\n    at fluid.containerForViewComponent (chrome-extension://gbenfaceipnpnobakclnelebaeinlelm/lib/infusion/dist/infusion-uio.js:137:228075)\n    at fluid.expander.invokeFunc [as typeFunc] (chrome-extension://gbenfaceipnpnobakclnelebaeinlelm/lib/infusion/dist/infusion-uio.js:137:116253)\n    at fluid.expandImmediateImpl (chrome-extension://gbenfaceipnpnobakclnelebaeinlelm/lib/infusion/dist/infusion-uio.js:137:113497)\n    at fluid.expandImmediate (chrome-extension://gbenfaceipnpnobakclnelebaeinlelm/lib/infusion/dist/infusion-uio.js:137:113252)\n    at fluid.memberFromRecord (chrome-extension://gbenfaceipnpnobakclnelebaeinlelm/lib/infusion/dist/infusion-uio.js:137:26907)"
}

jobara · 2022-12-20T14:31:13Z

On engadget.com when enabling Text-to-Speech I get the following error.

Error in event handler: Error: Assertion failure - check console for more details: More than one (32) container elements were found for selector article, [role~='article'], .article, #article with selector name article in context null
    at new fluid.FluidError (chrome-extension://gbenfaceipnpnobakclnelebaeinlelm/lib/infusion/dist/infusion-uio.js:128:2470)
    at fluid.builtinFail (chrome-extension://gbenfaceipnpnobakclnelebaeinlelm/lib/infusion/dist/infusion-uio.js:128:3038)
    at fluid.event.firer.fire (chrome-extension://gbenfaceipnpnobakclnelebaeinlelm/lib/infusion/dist/infusion-uio.js:128:25883)
    at fluid.fail (chrome-extension://gbenfaceipnpnobakclnelebaeinlelm/lib/infusion/dist/infusion-uio.js:128:3289)
    at fluid.container (chrome-extension://gbenfaceipnpnobakclnelebaeinlelm/lib/infusion/dist/infusion-uio.js:137:224673)
    at fluid.containerForViewComponent (chrome-extension://gbenfaceipnpnobakclnelebaeinlelm/lib/infusion/dist/infusion-uio.js:137:228075)
    at fluid.expander.invokeFunc [as typeFunc] (chrome-extension://gbenfaceipnpnobakclnelebaeinlelm/lib/infusion/dist/infusion-uio.js:137:116253)
    at fluid.expandImmediateImpl (chrome-extension://gbenfaceipnpnobakclnelebaeinlelm/lib/infusion/dist/infusion-uio.js:137:113497)
    at fluid.expandImmediate (chrome-extension://gbenfaceipnpnobakclnelebaeinlelm/lib/infusion/dist/infusion-uio.js:137:113252)
    at fluid.memberFromRecord (chrome-extension://gbenfaceipnpnobakclnelebaeinlelm/lib/infusion/dist/infusion-uio.js:137:26907)

jobara · 2022-12-20T19:36:09Z

src/js/enactors.js

+    domReaderContent: ["domReaderContent", "main", "article"],
+    controllerParentContainer: ["controllerParentContainer", "main", "article"],


I think we still need the "genericContent" option here which is likely defined in a parent grade. It provides more fallbacks to find the page content.

You are probably looking at an old version of this file. domReaderContent and controllerParentContainer are used by subcomponents domReader and controller. As these sub-components are now disabled for turning off the page level reading. These lines have been removed in the last commit.

jobara · 2022-12-20T19:37:50Z

.github/workflows/ci.yml

@@ -11,7 +11,7 @@ jobs:
    strategy:
      matrix:
        os: [ubuntu-latest, windows-latest, macOS-latest]
-        node-version: [14.x, 16.x]
+        node-version: [16.x]


Might as well add Node 18 now too.

Suggested change

node-version: [16.x]

node-version: [16.x, 18.x]

jobara · 2022-12-20T19:39:05Z

README.md

@@ -110,7 +99,7 @@ _**NOTE:** Published versions can be installed from the [Chrome Web Store](

 ### BSD 3-Clause

-* [Infusion v3.0.0-dev](https://fluidproject.org/infusion.html)
+* [Infusion v4.0.0](https://fluidproject.org/infusion.html)


Suggested change

* [Infusion v4.0.0](https://fluidproject.org/infusion.html)

* [Infusion v4.6.0](https://fluidproject.org/infusion.html)

jobara and others added 10 commits June 2, 2022 14:49

wip: update to manifest v3

db09e86

chore: cleanup suggested in code review

e1c856d

refactor: update dependencies

778e0f1

chore: more cleanup from review

89b7aac

fluid-projectGH-32: in progress of adding tests for background.js

b33d769

fluid-projectGH-32: add unit test for background.js

69c17a2

fluid-projectGH-32: add tests for enhancer.js using sinon-chrome

bacbfdf

fluid-projectGH-32: Finish writing tests and clean up

6402f55

fluid-projectGH-32: modify the npm build script

4565f2c

fluid-projectGH-32: update the node version used by Github CI

9f74561

cindyli requested review from jobara and amb26 December 8, 2022 17:45

cindyli mentioned this pull request Dec 8, 2022

feat!: update to manifest v3 (Resolves: #32, #33, #29) #39

Closed

amb26 reviewed Dec 17, 2022

View reviewed changes

cindyli added 2 commits December 19, 2022 09:08

fluid-projectGH-32: replace ; with && when defining npm tasks

6e0fef0

fluid-projectGH-32: add a code comment

fa68e7f

jobara reviewed Dec 20, 2022

View reviewed changes

fluid-projectGH-32: add sinon into devDependencies

12c6982

cindyli added 4 commits December 20, 2022 09:49

fluid-projectGH-32: defer javascript loading to fix no container error

ffb695d

fluid-projectGH-32: fix a couple of issues found in code review

c443e40

fluid-projectGH-32: fix the no container error at enabling UIO+

58a7f5f

fluid-projectGH-32: disable the page level reading

ebd7b55

jobara reviewed Dec 20, 2022

View reviewed changes

fluid-projectGH-32: address review suggestions

3df6fe2

jobara self-requested a review December 21, 2022 15:18

jobara approved these changes Dec 21, 2022

View reviewed changes

jobara merged commit cad0c64 into fluid-project:main Dec 21, 2022

github-actions bot mentioned this pull request Dec 21, 2022

chore(release): release 0.3.0 #44

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: update to manifest v3 (Resolves: #32, #33, #29) #40

feat: update to manifest v3 (Resolves: #32, #33, #29) #40

cindyli commented Dec 8, 2022

amb26 Dec 17, 2022

amb26 Dec 17, 2022

jobara Dec 19, 2022

cindyli Dec 19, 2022

jobara Dec 20, 2022

cindyli Dec 20, 2022

jobara commented Dec 20, 2022

jobara commented Dec 20, 2022

jobara Dec 20, 2022

cindyli Dec 20, 2022

jobara Dec 20, 2022

jobara Dec 20, 2022

		domReaderContent: ["domReaderContent", "main", "article"],
		controllerParentContainer: ["controllerParentContainer", "main", "article"],

	* [Infusion v4.0.0](https://fluidproject.org/infusion.html)
	* [Infusion v4.6.0](https://fluidproject.org/infusion.html)

feat: update to manifest v3 (Resolves: #32, #33, #29) #40

feat: update to manifest v3 (Resolves: #32, #33, #29) #40

Conversation

cindyli commented Dec 8, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jobara commented Dec 20, 2022

jobara commented Dec 20, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment