wsd: preprocess the per-user accessibility feature. #11854

hcvcastro · 2025-05-06T13:39:28Z

Each time the user requests the cool.html page
(an indirect request through an iframe), it will
include the user’s browser setting preferences.
In the case of accessibility, it will override the
global variable to enable the accessibility feature.

Change-Id: Ie56a4b3cc43e93cb7b72ede0ceaa184e3a438669
Signed-off-by: Henry Castro hcastro@collabora.com

browser/admin/src/integrator/AdminIntegratorSettings.ts

caolanm

I seem to find that every time I press the "save" or "reset" button I get a duplicate section?

Like this:

caolanm · 2025-05-08T16:43:17Z

browser/admin/src/integrator/AdminIntegratorSettings.ts

+		browserContainer.classList.add('section');
+
+		let elem = document.createElement('h3');
+		elem.textContent = _('Browser Settings');


Lets call this "View Settings" instead of "Browser Settings".

caolanm · 2025-05-08T16:43:53Z

browser/admin/src/integrator/AdminIntegratorSettings.ts

+		saveButton.type = 'button';
+		saveButton.id = 'xcu-save-button';
+		saveButton.classList.add('button', 'button-primary');
+		saveButton.title = _('Save Document settings');


And "Save View Settings"

caolanm · 2025-05-08T16:44:14Z

browser/admin/src/integrator/AdminIntegratorSettings.ts

+		resetButton.type = 'button';
+		resetButton.id = 'xcu-reset-button';
+		resetButton.classList.add('button', 'button--vue-secondary');
+		resetButton.title = _('Reset to default Document settings');


And: Reset to default View settings

caolanm · 2025-05-08T16:46:36Z

browser/admin/src/integrator/AdminIntegratorSettings.ts

 class SettingIframe {
 	private wordbook;
 	private xcuEditor;
+	private _browserSetting;


Can we rename browser/_browser to view/_view so its different to the existing browser settings. We want this as an additional thing separate from the existing "Interface" which contains the existing browser settings.

hcvcastro · 2025-05-09T14:38:38Z

Thank you. The renaming of labels has been completed.

caolanm · 2025-05-13T14:41:49Z

UI works for me now, no reload/save duplication problem.

Just need to download/apply these settings and if view accessibility is enabled then force the appropriate accessibiltyState flag to kit as on so that screen reading is automatically enabled when this setting was toggled on

meven · 2025-05-14T09:43:42Z

wsd/FileServer.cpp

+
    std::string enableAccessibility = stringifyBoolFromConfig(config, "accessibility.enable", false);
+    if (!userAccessibility.empty()) {
+        enableAccessibility = userAccessibility;


We should make sure userAccessibility can only be true|false, here it will inject any value it receives without any check.

enableAccessibility = "true";

For instance.

Yes, I believe the parseJSON does the prerequisite validation. Anyway, I will check it. Thanks.

caolanm · 2025-05-16T14:19:37Z

wsd/RequestVettingStation.cpp

                             "ClientRequestDispatcher and creating DocBroker for ["
                          << docKey << ']');

            _ws->sendMessage("browsersetting: null");


I imagine the intent was to put this inside the isUnitTesting and not duplicate it

Checking ..

The first is a raw WebSocket connection established before session creation, while the second uses the session instance, making them distinct. We can consider improvements after the unit tests pass.

caolanm · 2025-05-19T11:17:41Z

Since the "rename the 'accessibilityState'" commit the option does show in the settings page: I just get a blank section like this:

hcvcastro · 2025-05-19T11:35:42Z

Since the "rename the 'accessibilityState'" commit the option does show in the settings page: I just get a blank section like this:

Yes, please click reset to save the new values

Change-Id: I986c274213c07d5b7b81eb0cceca162fcbebd09e Signed-off-by: Henry Castro <hcastro@collabora.com>

Change-Id: I41da74292328345dea4f680134ce77995b72819c Signed-off-by: Henry Castro <hcastro@collabora.com>

It is the correct name to use in client-side JavaScript. Change-Id: I6c4a884de1c9ca82ff2573d277e4e9b07e2a289a Signed-off-by: Henry Castro <hcastro@collabora.com>

It is necessary because the system must wait for a server response (e.g., 'browsersetting:') to retrieve the user's data view settings before loading a document. Change-Id: If74ad7aa00a35e7ee0851b6ca887726651221c90 Signed-off-by: Henry Castro <hcastro@collabora.com>

Use the new 'browsersetting:' data to forward to the child session. Change-Id: Ic8bbdb18ee94f5eba94dc22fc9ecbefdd83831aa Signed-off-by: Henry Castro <hcastro@collabora.com>

Change-Id: I70779c13bed96454f19deaa07a11afb349f352c4 Signed-off-by: Henry Castro <hcastro@collabora.com>

: Send null browser setting data as soon as the WebSocket connection is upgraded, to load the document for the "file://" protocol. Change-Id: I0a479d6e5a3a757195accbd300ee33fd3705ba29 Signed-off-by: Henry Castro <hcastro@collabora.com>

Otherwise, our example file "debug.html" will not work. Change-Id: I6b37ade12ddec545587a088d2c671631b3818812 Signed-off-by: Henry Castro <hcastro@collabora.com>

It should be processed uniquely during the very early WebSocket connection opening and should not be queued. Change-Id: Iaca25ba81da5287d9c45ef0c6e01b59197b7ef2c Signed-off-by: Henry Castro <hcastro@collabora.com>

It must be initialized when the server sends 'browsersetting:' data; otherwise, the input will have the wrong class type upon initialization. Change-Id: I5226e92c4be921d93dd6c9e236db4bf428c1c8a5 Signed-off-by: Henry Castro <hcastro@collabora.com>

Our C++ unit tests handle manual URL loading. It’s better not to disrupt the progress of loading messages. Change-Id: I1446fc230938a6ecd86892496e51430acc112a2d Signed-off-by: Henry Castro <hcastro@collabora.com>

undefined empty() Change-Id: Id008dbc42d751b22e8557030dc760e9f6990f3bc Signed-off-by: Henry Castro <hcastro@collabora.com>

Loading the URL document takes a little longer than expected. Adjust the timeout accordingly. Change-Id: Ib9a7150c81b4386aa84cf49d95cc1588e1fe32c4 Signed-off-by: Henry Castro <hcastro@collabora.com>

eszkadev · 2025-05-20T05:35:56Z

cypress_test/integration_tests/common/helper.js

 function documentChecks(skipInitializedCheck = false) {
 	cy.log('>> documentChecks - start');

-	cy.cGet('#document-canvas', {timeout : Cypress.config('defaultCommandTimeout') * 2.0});


Allowing document to load 300s seems to be too much (default is 10s * 2 IIRC).

CI is in a good shape now so not a problem.
When we can't get canvas there is most often problem on the server side, some error which kills websocket or similar. Did you try to run this locally and debug? make run-desktop in cypress_test directory. See logs in the browser, requests. Also there are error.log in workdir which might helpful to see.

I was testing the values, but it still fails. I tried running it locally with run-mobile spec=something, and it loads the canvas and starts testing the unit tests. However, these changes should not affect loading the JavaScript bundle or initializing the map. Interestingly, I will start adding logs to identify the issue.

caolanm · 2025-05-20T08:05:08Z

I think the admin ui stuff is good, but it looks fairly invasive for the browser side. I have a thought about how to make this a more minimal.

github-project-automation bot added this to Collabora Online May 6, 2025

github-project-automation bot moved this to To Review in Collabora Online May 6, 2025

hcvcastro force-pushed the pr/master/AD branch from 08913f8 to f031660 Compare May 7, 2025 15:35

github-advanced-security bot found potential problems May 7, 2025

View reviewed changes

browser/admin/src/integrator/AdminIntegratorSettings.ts Fixed Show fixed Hide fixed

browser/admin/src/integrator/AdminIntegratorSettings.ts Fixed Show fixed Hide fixed

hcvcastro force-pushed the pr/master/AD branch 2 times, most recently from 4acdc08 to 6c9413a Compare May 8, 2025 11:18

caolanm reviewed May 8, 2025

View reviewed changes

hcvcastro force-pushed the pr/master/AD branch from 6c9413a to 775abdb Compare May 9, 2025 14:36

hcvcastro force-pushed the pr/master/AD branch 2 times, most recently from bcb3e2c to c393887 Compare May 13, 2025 11:05

meven reviewed May 14, 2025

View reviewed changes

hcvcastro force-pushed the pr/master/AD branch 5 times, most recently from 8818b5d to 2663f3d Compare May 16, 2025 13:30

caolanm reviewed May 16, 2025

View reviewed changes

hcvcastro force-pushed the pr/master/AD branch from 2663f3d to 00e46d3 Compare May 19, 2025 10:56

hcvcastro force-pushed the pr/master/AD branch 2 times, most recently from 0b8b81d to 3ff26b1 Compare May 19, 2025 14:31

hcvcastro added 6 commits May 19, 2025 10:51

browser: admin: generate the browser settings UI.

25b3590

Change-Id: I986c274213c07d5b7b81eb0cceca162fcbebd09e Signed-off-by: Henry Castro <hcastro@collabora.com>

browser: admin: remove container to prevent duplicate UI elements

413a43e

Change-Id: I41da74292328345dea4f680134ce77995b72819c Signed-off-by: Henry Castro <hcastro@collabora.com>

browser: admin: rename the 'accessibilityState' data

8ab3c18

It is the correct name to use in client-side JavaScript. Change-Id: I6c4a884de1c9ca82ff2573d277e4e9b07e2a289a Signed-off-by: Henry Castro <hcastro@collabora.com>

wsd: fix the 'accessibilityState' data forwarding to the child session.

d49a875

Use the new 'browsersetting:' data to forward to the child session. Change-Id: Ic8bbdb18ee94f5eba94dc22fc9ecbefdd83831aa Signed-off-by: Henry Castro <hcastro@collabora.com>

browser: fix an unhandled exception during initialization.

f72988e

Change-Id: I70779c13bed96454f19deaa07a11afb349f352c4 Signed-off-by: Henry Castro <hcastro@collabora.com>

hcvcastro added 5 commits May 19, 2025 10:51

wsd: wopi: send null browserSetting if the operation is not successful

89b3f43

Otherwise, our example file "debug.html" will not work. Change-Id: I6b37ade12ddec545587a088d2c671631b3818812 Signed-off-by: Henry Castro <hcastro@collabora.com>

browser: ensure unique 'browserSetting' processing.

f506ec6

It should be processed uniquely during the very early WebSocket connection opening and should not be queued. Change-Id: Iaca25ba81da5287d9c45ef0c6e01b59197b7ef2c Signed-off-by: Henry Castro <hcastro@collabora.com>

wsd: fix C++ unit test assertion failures.

31720f9

Our C++ unit tests handle manual URL loading. It’s better not to disrupt the progress of loading messages. Change-Id: I1446fc230938a6ecd86892496e51430acc112a2d Signed-off-by: Henry Castro <hcastro@collabora.com>

hcvcastro force-pushed the pr/master/AD branch from 3ff26b1 to 31720f9 Compare May 19, 2025 14:52

browser: mobile: fix the unhandled exception

9ed9cd0

undefined empty() Change-Id: Id008dbc42d751b22e8557030dc760e9f6990f3bc Signed-off-by: Henry Castro <hcastro@collabora.com>

hcvcastro force-pushed the pr/master/AD branch 2 times, most recently from fc308f1 to f213630 Compare May 19, 2025 19:04

cypress: increase the load command timeout

b0c2d56

Loading the URL document takes a little longer than expected. Adjust the timeout accordingly. Change-Id: Ib9a7150c81b4386aa84cf49d95cc1588e1fe32c4 Signed-off-by: Henry Castro <hcastro@collabora.com>

hcvcastro force-pushed the pr/master/AD branch from f213630 to b0c2d56 Compare May 19, 2025 20:07

eszkadev reviewed May 20, 2025

View reviewed changes

caolanm changed the base branch from master to private/caolan/view_settings_a11y May 20, 2025 13:20

caolanm merged commit 08abf22 into CollaboraOnline:private/caolan/view_settings_a11y May 20, 2025
10 of 13 checks passed

github-project-automation bot moved this from To Review to Done in Collabora Online May 20, 2025

wsd: preprocess the per-user accessibility feature. #11854

wsd: preprocess the per-user accessibility feature. #11854

Uh oh!

Conversation

hcvcastro commented May 6, 2025

Uh oh!

Uh oh!

Uh oh!

caolanm left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

hcvcastro commented May 9, 2025

Uh oh!

caolanm commented May 13, 2025

Uh oh!

meven May 14, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

caolanm commented May 19, 2025

Uh oh!

hcvcastro commented May 19, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

caolanm commented May 20, 2025

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

meven May 14, 2025 •

edited

Loading