Will likely become worse with T248162: Use revision and segment ID as input for synthesizing speech

Decision is to break out 2 as a later task but implement 1 in the near future

Adding another condition to https://gerrit.wikimedia.org/r/plugins/gitiles/mediawiki/extensions/Wikispeech/+/master/WikispeechHooks.php#87 should solve this.

In T254864#6214065, @Lokal_Profil wrote:

Decision is to break out 2 as a later task but implement 1 in the near future

Suggestion 2 (Define a default message which gets synthesized and played) was broken out as T255112: ☂ Synthezise default error messages. Updating the task description to remove that

In what log should this be seen: TypeError: utterance is undefined? I can't find that error.

Looks like that's from the Javascript console.

One longterm solution could perhaps be:

When a non existing page appears: then create something similar to: ext.wikispeech.ui.js::this.addDialogs, and decide which of the pre-synthesized messages ( T387284 ) we want to use for this occasion - when there is a non-existing page.

Change #1143994 had a related patch set uploaded (by Viktoria Hillerud WMSE; author: Viktoria Hillerud WMSE):

[mediawiki/extensions/Wikispeech@master] Play error message if non-existing page is visited

https://gerrit.wikimedia.org/r/1143994

Theese are the messages:

"noarticletext"
"noarticletext-nopermission"
"newarticletext"

This is dependent on T255112, and when that patch is done, I can start figuring out how to play and show the relevant error message in the GUI.

The current patch adds a new message and shows a dialogue with audio. I think it would be better if you can use the message that's on the screen and play it using the player. Did you look into that?

In T254864#11110227, @Sebastian_Berlin-WMSE wrote:

The current patch adds a new message and shows a dialogue with audio. I think it would be better if you can use the message that's on the screen and play it using the player. Did you look into that?

I see, I didn't think of that, but I can look into it.

I have a solution now, but the highlighter doesn't work, maybe its a task for later?

In the last patch you get the text in the frontend. This was supposed to use the reading messages functionality in T255112: ☂ Synthezise default error messages. Was there an issue that made you abandon that? If so it should be noted (and probably a new task should be created to solve it in the future).

In T254864#11166094, @Sebastian_Berlin-WMSE wrote:

In the last patch you get the text in the frontend. This was supposed to use the reading messages functionality in T255112: ☂ Synthezise default error messages. Was there an issue that made you abandon that? If so it should be noted (and probably a new task should be created to solve it in the future).

Ah... That makes sense. This task has been ongoing for so long that I didn't really remember this, you mean for it to use the functionality made in this patch?: https://phabricator.wikimedia.org/T387280

Yes.

This will be WIP and refactored when T407474 is done, since there will be a new system for the wikispeech-listen api to handle messages that we can use in this patch.

I have implemented the logic using the API to send message-key from frontend, the change is WIP on gerrit: Play error message if non-existing page is visited

Though, I am running into an issue, when playing the second segment of the message-key noarticletext, the first audio segment is played correctly (reads: “There is currently no text in this page”).
But, the second segment is being read out as a long sequence of technical terms, e.g. “Special colon Log page, forwardslash, less than, span, equals, plainlinks...” etc.

I think this is because of that when sending a message key to ApiWikispeechListen:execute(), it doesn't go through the same chain as regular utterances do regarding removeTags.

Could this suspision be correct? @Sebastian_Berlin-WMSE
If that's the case, then I a think I should correct it in a separate task/patch.

It actually was just a small piece of code needed, just a few lines, so I think I'll stick with just including that in this patch

It looks like you've recreated some of the logic for playing utterances. It would be better if you could re-use what we already have.

This would also make it less likely to cause issues. For instance now when I click play on an error page it changes back to play after the first sentence has been read. Then I can play again and have two overlapping utterance playing. Also I can't stop the first one.

Do you mean in storage.js?

Yes. Ideally you can use Storage.loadUtterances() and just change the API parameters. May need to make two functions that calls loadUtterances(), one for content and one for messages.

I explored the approach of reusing Storage.loadUtterances() with separate functions for content and message utterances, howver I couldn't get that to work, and it took a very long time until I decided to take on another approach. Since rror messages (messagekey utterances) doesn't behave the same as content utterances, we need separate handling.

So instead of trying to make message key and audio error handling fit into the same pattern as normal utterances, I generate separate errorUtterances objects for each audio segment in the response and store them in the utterance. Then in the player, I map over these errorUtterances, and making sure only one is played at a time and avoiding the overlapping playback.

Also, skipBackUtterance can’t work for messagekey, because there’s no previous utterance to go back to, since theyr'e not normal utterances.

I can play and pause, but the rest of the navigation is broken. Skipping by sentence just stops. Skipping by word causes errors like:

11:47:31.899 Uncaught TypeError: can't access property "utterance", originalToken is null
    getPreviousToken ext.wikispeech.storage.js:598
    skipBackToken ext.wikispeech.player.js:369
    addControlPanel ext.wikispeech.ui.js:87
    emit oojs.js:858
    onClick oojs-ui-core.js:2381
    jQuery 9
    setButtonElement oojs-ui-core.js:2317
    OoUiMixinButtonElement oojs-ui-core.js:2260
    OoUiButtonWidget oojs-ui-core.js:3974
    addButton ext.wikispeech.ui.js:218
    addControlPanel ext.wikispeech.ui.js:85
    init ext.wikispeech.ui.js:25
    init ext.wikispeech.main.js:56
    js ext.wikispeech.main.js:112
    jQuery 3
    using mediawiki.base.js:873
    doPropagation startup.js:769
    requestIdleCallback handler*setAndPropagate startup.js:826
    markModuleReady startup.js:1246
    runScript startup.js:1292
    cssHandle startup.js:1356
    flushCssBuffer startup.js:602
    addEmbeddedCSS startup.js:633
    execute startup.js:1370
    doPropagation startup.js:753
    requestIdleCallback handler*setAndPropagate startup.js:826
    markModuleReady startup.js:1246
    runScript startup.js:1280
    cssHandle startup.js:1356
    flushCssBuffer startup.js:602
    addEmbeddedCSS startup.js:633
    execute startup.js:1370
    doPropagation startup.js:753
    requestIdleCallback handler*setAndPropagate startup.js:826
    markModuleReady startup.js:1246
    runScript startup.js:1302
    cssHandle startup.js:1356
    flushCssBuffer startup.js:602
    addEmbeddedCSS startup.js:633
    execute startup.js:1370
    doPropagation startup.js:753
    requestIdleCallback handler*setAndPropagate startup.js:826
    impl startup.js:2015
    <anonymous> load.php:1
    indirectEval startup.js:1167
    asyncEvalTask startup.js:1646
    asyncEval startup.js:1625
    requestIdleCallback handler*asyncEval startup.js:1624
    asyncEvalTask startup.js:1642
    asyncEval startup.js:1625
    requestIdleCallback handler*asyncEval startup.js:1624
    work startup.js:1800
    enqueue startup.js:1224
    load startup.js:2061
    <anonymous> startup.js:2600
    <anonymous> startup.js:2641
3 ext.wikispeech.storage.js:598:17

Ah, yes, I made with purpose that because I didn't think it was necessary to have those functions on a non existing page. But I understand that that is not a desired behavior, and that it can be done a bit smarter. I'll look over it.

I changed the handling of error utterances so that token navigation works the same way as for normal utterances. But instead I handle tokens for error utterances when they’re created, since these don’t fit the same structure as regular utterances.
Playback and navigation for error utterances are now handled separately in an own function because error messages don’t behave like normal utterances as error messages are not part of the main utterance array.

Here is several updates about this patch:

• After switching from .done() to .then() in storage.js, several QUnit tests started failing. The reason seems to be that the tests were using $.Deferred().resolve(), which doesn’t seem to fully support promise methods like .then() in all cases. To fix this, I updated the tests to use real promises with Promise.resolve() instead. I also made the test functions async and used await where necessary. This solved the test failures. This is probably something we will have to end up doing across all of our modules and QUnit tests, that gives linting warnings regarding using .then() to .done()

• When we talked about the loadUtterances(), we discovered that the .fail catches a broader set of failures rather than just checking for missingtitle. The issues if it fails to loadUtterances could be many more situations, so we have to explicit check for .fail (now .catch) where the title doesn’t exist, since it is what this task is about. We then talked about putting the logic in main.js in init() but I have run into some questions regarding that: If we check for missingtitle in main.js, aren’t we just duplicating logic that storage already handles? For example sending an api-request? It feels kind of better to let storage manage errors and loadFailed, so main.js doesn’t have to worry about the details. What do you think?

• Also, the loadAudio() was quite complex, and not very readable, therefore I created help functions and broke out big chunks of code.

Sounds good.

In T254864#11597045, @Viktoria_Hillerud_WMSE wrote:

If we check for missingtitle in main.js, aren’t we just duplicating logic that storage already handles? For example sending an api-request? It feels kind of better to let storage manage errors and loadFailed, so main.js doesn’t have to worry about the details. What do you think?

I think that's reasonable. This is probably one of those times it's better to do the simpler solution now and change it later as needed.

I tested the new patch and found an error when you play, pause, and then play again. The first utterance is read, but then it stop. The browser console shows the errors below. The first one is when you start playing again and the second is when the first utterance finishes.

09:43:38.781 Navigated to http://localhost:8080/wiki/Apa?debug=1
09:43:38.798 Reloaded
09:43:38.818 GET
http://localhost:8080/wiki/Apa?debug=1
[HTTP/1.1 404 Not Found 100ms]

09:43:39.149 GET
http://localhost:8080/favicon.ico
[HTTP/1.1 404 Not Found 0ms]

09:43:43.574 Uncaught TypeError: can't access property "map", token.items is undefined
    highlightToken ext.wikispeech.highlighter.js:178
    startTokenHighlighting ext.wikispeech.highlighter.js:154
    play ext.wikispeech.player.js:122
    playOrPause ext.wikispeech.player.js:34
    playPauseButton ext.wikispeech.ui.js:102
    emit oojs.js:858
    onClick oojs-ui-core.js:2381
    jQuery 9
    setButtonElement oojs-ui-core.js:2317
    OoUiMixinButtonElement oojs-ui-core.js:2260
    OoUiButtonWidget oojs-ui-core.js:3974
    addButton ext.wikispeech.ui.js:272
    addControlPanel ext.wikispeech.ui.js:100
    init ext.wikispeech.ui.js:25
    init ext.wikispeech.main.js:57
    js ext.wikispeech.main.js:113
    jQuery 3
    using mediawiki.base.js:873
    doPropagation startup.js:769
    requestIdleCallback handler*setAndPropagate startup.js:826
    markModuleReady startup.js:1246
    runScript startup.js:1292
    cssHandle startup.js:1356
    flushCssBuffer startup.js:602
    addEmbeddedCSS startup.js:633
    execute startup.js:1370
    doPropagation startup.js:753
    requestIdleCallback handler*setAndPropagate startup.js:826
    markModuleReady startup.js:1246
    runScript startup.js:1280
    cssHandle startup.js:1356
    flushCssBuffer startup.js:602
    addEmbeddedCSS startup.js:633
    execute startup.js:1370
    doPropagation startup.js:753
    requestIdleCallback handler*setAndPropagate startup.js:826
    markModuleReady startup.js:1246
    runScript startup.js:1302
    cssHandle startup.js:1356
    flushCssBuffer startup.js:602
    addEmbeddedCSS startup.js:633
    execute startup.js:1370
    doPropagation startup.js:753
    requestIdleCallback handler*setAndPropagate startup.js:826
    impl startup.js:2015
    <anonymous> jQuery
load.php:5:354
09:43:45.303 Uncaught TypeError: can't access property "length", this.errorUtteranceList is null
    onended ext.wikispeech.player.js:164
    playCurrentErrorUtterance ext.wikispeech.player.js:162
    play ext.wikispeech.player.js:132
    jQuery 17
    xhr index.js:333
    jQuery 6
    ajax index.js:305
    get index.js:159
    requestMessageUtteranceTts ext.wikispeech.storage.js:317
    loadAudio ext.wikispeech.storage.js:170
    prepareUtterance ext.wikispeech.storage.js:146
    play ext.wikispeech.player.js:128
    playOrPause ext.wikispeech.player.js:34
    playPauseButton ext.wikispeech.ui.js:102
    emit oojs.js:858
    onClick oojs-ui-core.js:2381
    jQuery 9
    setButtonElement oojs-ui-core.js:2317
    OoUiMixinButtonElement oojs-ui-core.js:2260
    OoUiButtonWidget oojs-ui-core.js:3974
    addButton ext.wikispeech.ui.js:272
    addControlPanel ext.wikispeech.ui.js:100
    init ext.wikispeech.ui.js:25
    init ext.wikispeech.main.js:57
    js ext.wikispeech.main.js:113
    fire jQuery
load.php:9:265

I’ve been able to reproduce the issue with errorUtteranceList becoming null after play, pause, play again. That was caused by state being reset while an onended callback was still active, so that is fixed now!
However, I haven’t been able to reproduce the token.items is undefined error on my side, don't know if it had something to do with a rebase I recently did, and you have the latest version of the patch?

I have the latest patchset, just the line has changed to 175 (possibly due to your changes). It happens during highlighting. Do you have that enabled?

I see! I had forgotten to tell the highlighter that if it is an errorUtterance, then it should not try to highlight anything. Similar to partOfContent

Change #1143994 merged by jenkins-bot:

[mediawiki/extensions/Wikispeech@master] Play error message if non-existing page is visited

https://gerrit.wikimedia.org/r/1143994