When requesting formatting information, default colors are now explicitely reported in Wordpad and other edit fields by CyrilleB79 · Pull Request #13942 · nvaccess/nvda

CyrilleB79 · 2022-07-24T20:58:22Z

Link to issue number:

Summary of the issue:

In Wordpad or in log viewer, pressing NVDA+F reports "default color on default color" as color information in formatting information. This information is of no use to know the color really displayed, what may be necessary when working with sighted persons.

In log viewer and other system edit fields, default colors are the ones provided by the system and can vary according to system settings, e.g. enabling high contrast theme.
In Wordpad, "Default color" is meant to be the one called "Automatic" and corresponds to system color.

Description of user facing changes

When pressing NVDA+F in Wordpad or NVDA log viewer, NVDA reports the text / background color and in addition, indicates if it is the default color, i.e. system color.
- I have not bothered to report the application's color "Automatic" in Wordpad instead of "default" because it would probably need to create a WordPad appModule and specific processing just for this distinction; I think it was not worth it.
In MS Word, when the text color is set to "Automatic", "Automatic color" is reported instead of "default color". Indeed, this matches Word application terminology; in Word, automatic color and default color are two distinc concepts:
- "automatic" is a color which adapts to favorise a sufficient contrast with the background
- "default color" is the color of the text when you open a new document (if registered in normal.dot) or when you remove formatting (when registered in the current document only).

Description of development approach

According to the CHARFORMATW structure for the class EditTextInfo and to ITextFont::GetForeColor for the class ITextDocumentTextInfo, the system colors are used for default colors. Thus, I retrieve these colors in these cases.

Testing strategy:

Manual tests:

For the class ITextDocumentTextInfo:

Tested NVDA+F in WordPad (default automatic color) and log viewer
Activated high contrast and changed the thems to check that the colors changed as visually.

For the class EditTextInfo, I have performed the same tests, making previously the changes in #11897 (comment).

Known issues with pull request:

In Word, I have not found a way to know the real color of the text when automatic is selected; thus only this information is provided.
When UIA is enabled in Word, the true colors are reported, but we do not have the information if it corresponds to "automatic" selected in the application or if a "true" color is selected.

Change log entries:

Bug fixes
When requesting formatting information, colors are now explicitly reported in Wordpad or log viewer, rather than only "Default color". (#13959)

Code Review Checklist:

Pull Request description:
- description is up to date
- change log entries
Testing:
- Unit tests
- System (end to end) tests
- Manual testing
API is compatible with existing add-ons.
Documentation:
- User Documentation
- Developer / Technical Documentation
- Context sensitive help for GUI changes
UX of all users considered:
- Speech
- Braille
- Low Vision
- Different web browsers
- Localization in other languages / culture than English

…itely reported in Wordpad and other edit fields.

AppVeyorBot · 2022-07-24T21:46:48Z

PASS: Translation comments check.
PASS: Unit tests.
PASS: Lint check.
FAIL: System tests. See test results for more information.
Build (for testing PR): https://ci.appveyor.com/api/buildjobs/oe9kf6em2ktfgwah/artifacts/output/nvda_snapshot_pr13942-25968,85d724fa.exe

See test results for failed build of commit 85d724fa09

CyrilleB79 · 2022-07-25T20:51:23Z

I realize that the issue is broader since "Default color" is also reported in MS Word when the color is "Automatic".

Converting to draft while investigating further.

lukaszgo1

Since there at least two other places (support for pre UIA dropdowns in Excel, and retrieval of highlight colors in display model) where GetSysColor is used it makes sens to create a wrapper around it in winUser, and an IntEnum which defines values representing known display elements this function accepts.

CyrilleB79 · 2022-07-27T21:41:56Z

I have opened #13959 to discuss how default colors need to be reported more generally.

…te automatic instead of default.

CyrilleB79 · 2022-09-20T08:53:39Z

I have updated the code and the initial description taking into account what was discussed in #13959 (Cc @feerrenrut).

I still have to create a wrapper for GetSysColor as suggested by @lukaszgo1, I guess in the new winAPI/winUser/__init__.py file. Then this PR will be ready for review again.

CyrilleB79 · 2022-09-26T14:16:24Z

This PR is now ready to review again.

Note for those who have reviewed it at the beginning: Please re-read the initial description that have been modified.

Also, I have made a wrapper for GetSysColor as per #13942 (review).

In the SysColorIndex enum, I have only added the items for the values we use; let me know if the recommended way to do is to already integrate all the possible items instead, even those which we do not use yet.
Regarding tests, I have only tested the log viewer. How can I test other uses of GetSysColor (Excel, displayModel?)

AppVeyorBot · 2022-09-26T15:18:01Z

PASS: Translation comments check.
PASS: Unit tests.
PASS: Lint check.
FAIL: System tests (tags: installer NVDA). See test results for more information.
Build (for testing PR): https://ci.appveyor.com/api/buildjobs/qqjkr7r0j9df8wwt/artifacts/output/nvda_snapshot_pr13942-26686,becfe662.exe
CI timing (mins):
INIT 0.0,
INSTALL_START 0.8,
INSTALL_END 0.9,
BUILD_START 0.0,
BUILD_END 25.4,
TESTSETUP_START 0.0,
TESTSETUP_END 0.3,
TEST_START 0.0,
TEST_END 14.5,
FINISH_END 0.1

See test results for failed build of commit becfe662f3

lukaszgo1 · 2022-09-26T18:08:56Z

* Regarding tests, I have only tested the log viewer. How can I test other uses of GetSysColor (Excel, displayModel?)

When testing default selection colors in DisplayModel I personally use Fast Log Entry - there is nothing special about it other than its main edit control has to be accessed via Display Hooks, and it uses default system colors for selection. I've tested the build from this PR, and can confirm that selections can still be reported in display model.
The Drop downs in Excel rely on display hooks in 2010 and earlier (possibly also in 2013 but don't quote me on that). Since it is likely you don't have easy access to legacy versions of Office I've also tested that with this PR drop downs in Excel 2010 behave as expected.

CyrilleB79 · 2022-09-26T20:43:15Z

Many thanks @lukaszgo1 for these tests.

seanbudd

Generally looks good to me, thanks @CyrilleB79

seanbudd

Thanks @CyrilleB79 , looks almost ready

seanbudd

Thanks @CyrilleB79

CyrilleB79 added 3 commits July 24, 2022 22:27

When requesting formatting information, default colors are now explic…

3cf08a4

…itely reported in Wordpad and other edit fields.

Linting

5e2b71f

Remove useless lines.

8db3d2f

CyrilleB79 marked this pull request as ready for review July 25, 2022 05:08

CyrilleB79 requested a review from a team as a code owner July 25, 2022 05:08

CyrilleB79 requested a review from seanbudd July 25, 2022 05:08

seanbudd added this to the 2022.4 milestone Jul 25, 2022

CyrilleB79 marked this pull request as draft July 25, 2022 20:49

lukaszgo1 reviewed Jul 26, 2022

View reviewed changes

CyrilleB79 mentioned this pull request Jul 27, 2022

A color reported as "default" gives no information #13959

Closed

seanbudd removed this from the 2022.4 milestone Aug 31, 2022

CyrilleB79 added 2 commits September 19, 2022 22:51

Merge branch 'master' into reportDefaultColor

9d3fb91

Wordpad/NVDALog: indicate true color as well as default; Word: indica…

0470b04

…te automatic instead of default.

CyrilleB79 added 2 commits September 26, 2022 16:02

Make a common wrapper for GetSysColor.

7e6c650

Merge branch 'master' into reportDefaultColor

77810d6

CyrilleB79 marked this pull request as ready for review September 26, 2022 14:17

CyrilleB79 requested review from lukaszgo1 and removed request for seanbudd September 26, 2022 14:18

lukaszgo1 approved these changes Sep 26, 2022

View reviewed changes

CyrilleB79 closed this Sep 26, 2022

CyrilleB79 reopened this Sep 26, 2022

seanbudd self-requested a review September 26, 2022 23:54

seanbudd added the blocked/needs-code-review label Sep 26, 2022

seanbudd reviewed Sep 27, 2022

View reviewed changes

Comment thread source/NVDAObjects/window/winword.py

Comment thread source/NVDAObjects/window/edit.py Outdated

Comment thread source/winAPI/winUser/__init__.py Outdated

seanbudd removed the blocked/needs-code-review label Sep 27, 2022

seanbudd marked this pull request as draft September 29, 2022 05:42

CyrilleB79 added 2 commits October 6, 2022 10:32

Address review

c118f3b

Merge branch 'master' into reportDefaultColor

1c89b71

CyrilleB79 marked this pull request as ready for review October 6, 2022 09:03

seanbudd reviewed Oct 7, 2022

View reviewed changes

Comment thread source/NVDAObjects/window/edit.py Outdated

Comment thread source/NVDAObjects/window/edit.py Outdated

Add type hints

bc48878

CyrilleB79 requested a review from seanbudd October 7, 2022 07:37

seanbudd approved these changes Oct 9, 2022

View reviewed changes

seanbudd added 2 commits October 10, 2022 16:31

Merge remote-tracking branch 'origin/master' into reportDefaultColor

e9d6136

update changes

cba0caa

seanbudd merged commit de86749 into nvaccess:master Oct 10, 2022

nvaccessAuto added this to the 2023.1 milestone Oct 10, 2022

CyrilleB79 deleted the reportDefaultColor branch October 10, 2022 19:57

Uh oh!

Conversation

CyrilleB79 commented Jul 24, 2022 • edited by seanbudd Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Link to issue number:

Summary of the issue:

Description of user facing changes

Description of development approach

Testing strategy:

Known issues with pull request:

Change log entries:

Code Review Checklist:

Uh oh!

AppVeyorBot commented Jul 24, 2022

Uh oh!

CyrilleB79 commented Jul 25, 2022

Uh oh!

lukaszgo1 left a comment

Choose a reason for hiding this comment

Uh oh!

CyrilleB79 commented Jul 27, 2022

Uh oh!

CyrilleB79 commented Sep 20, 2022

Uh oh!

CyrilleB79 commented Sep 26, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

AppVeyorBot commented Sep 26, 2022

Uh oh!

lukaszgo1 commented Sep 26, 2022

Uh oh!

CyrilleB79 commented Sep 26, 2022

Uh oh!

seanbudd left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

seanbudd left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

seanbudd left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

CyrilleB79 commented Jul 24, 2022 •

edited by seanbudd

Loading

CyrilleB79 commented Sep 26, 2022 •

edited

Loading