Skip to content

NVDAObjects/UIA: recognize UWP tooltips#8124

Merged
feerrenrut merged 3 commits into
nvaccess:masterfrom
josephsl:i8118uwpTooltips
Jul 26, 2019
Merged

NVDAObjects/UIA: recognize UWP tooltips#8124
feerrenrut merged 3 commits into
nvaccess:masterfrom
josephsl:i8118uwpTooltips

Conversation

@josephsl

Copy link
Copy Markdown
Contributor

Link to issue number:

Fixes #8118

Summary of the issue:

Recognize UWP/XAML tooltips such as ones from Microsoft Edge as proper tooltips.

Description of how this pull request fixes the issue:

Tooltips from universal apps are powered by XAML and has a specific class name (for now). Prior to this, these tooltips were not announced by NVDA. Cases include Edge's Settings button tooltip, which displays hotkey information (Alt+X) in Insider Preview builds. Thus recognize them as proper tooltips through an overlay class powered by NVDAObjects.behaviors.ToolTip.

Testing performed:

Tested Edge on Windows 10 Versions 1703 (Creators Update), 1709 (Fall Creators Update) and 1803 preview (Spring Creators Update).

Known issues with pull request:

None at this time.

Change log entry:

Bug fixes: In Windows 10, NVDA will announce tooltips from universal apps if NVDA is configured to report tooltips in object presentation dialog.

@josephsl josephsl requested a review from michaelDCurran March 25, 2018 17:45
@LeonarddeR

Copy link
Copy Markdown
Collaborator

@josephsl commented on 25 mrt. 2018 19:44 CEST:

Tooltips from universal apps are powered by XAML and has a specific class name (for now).

What UIA control type do these have? It would be nice if we could also support WPF tooltips this way, though I have no idea where to find these for testing purposes.

@josephsl

josephsl commented Mar 26, 2018 via email

Copy link
Copy Markdown
Contributor Author

@josephsl

Copy link
Copy Markdown
Contributor Author

Hi,

The tooltips are recognized as tooltips. Thanks.

@LeonarddeR

Copy link
Copy Markdown
Collaborator

@josephsl commented on 26 mrt. 2018 09:21 CEST:

Hi,

The tooltips are recognized as tooltips. Thanks.

In that case, why not just treat every UIA tooltip as tooltip?

@josephsl

josephsl commented Mar 26, 2018 via email

Copy link
Copy Markdown
Contributor Author

@LeonarddeR

Copy link
Copy Markdown
Collaborator

@josephsl commented on 26 mrt. 2018 16:48 CEST:

Hi, not yet, as one of them is volume slider tooltip, which is annoying. Thanks.

Could you please elaborate?

@josephsl

josephsl commented Mar 26, 2018 via email

Copy link
Copy Markdown
Contributor Author

@LeonarddeR

Copy link
Copy Markdown
Collaborator

@josephsl commented on 26 Mar 2018, 17:44 CEST:

Hi, when you change audio volume, a tooltip comes up and fires tooltip open event. Although the framework is different, it is still annoying.

I think many people consider tool tips to be annoying. I prefer consistency over subjective decisions about what is considered annoying and what is not. @michaelDCurran: Thoughts?

@michaelDCurran

Copy link
Copy Markdown
Member

I tend to agree with @LeonarddeR. If a choose to have tooltips reported, then I want to hear all tooltips. Obviously this can get annoying, thus why it is disabled by default in NVDA.

@josephsl

Copy link
Copy Markdown
Contributor Author

Hi,

Testing shows that I had incomplete assumptions: tooltip for volume slider is fired infrequently, thus I'll follow above recommendations.

Thanks.

@josephsl

Copy link
Copy Markdown
Contributor Author

Hi,

Note that new UIA tool tip handling works properly on Windows 8 and later. An additional benefit is that NVDA will now also announce volume changes (if that is announced, that is).

Thanks.

@Robert-J-H

Copy link
Copy Markdown
Contributor

Hi
I must say that the Volume Level tooltip is very distracting.
Although it closes essentially #5726, the opinions uttered there aren't by no way unanimous.

It is especially annoying if you adjust the volume during say all.
It might happen that it is announced much later (after e.g. a paragraph).

It is not very user-friendly if the possible decision for tool tips is
"Hav'em all or none".
It is a special case tool tip because:

  1. It gives the immediate result for a user input
  2. The effect can also be heard by other means (playing audio, running speech)
    As a musician, I clearly prefer the second way.
    More so as the user intentionally lowers or rises the volume because of what she/he heard or didn't hear and not because the numeric value wasn't to her/his liking.
    In other words, the cause/effect chain is broken or at least forked into two outputs.
    It would be great to have this tool tip only announced for when the audio device is idle.
    Does this make sense? ;-)

@LeonarddeR

Copy link
Copy Markdown
Collaborator

@Robert-J-H commented on 21 Apr 2018, 08:45 CEST:

It is especially annoying if you adjust the volume during say all.
It might happen that it is announced much later (after e.g. a paragraph).

May be this is more a general problem of reporting tooltips during say all, not specifically to the volume tool tip.

It is not very user-friendly if the possible decision for tool tips is
"Hav'em all or none".

What do you suggest otherwise? I don't think it is user friendly either to distinguish between several tool tips the user does not know about.

@josephsl

josephsl commented Apr 21, 2018 via email

Copy link
Copy Markdown
Contributor Author

@Robert-J-H

Copy link
Copy Markdown
Contributor

Yes, the user should be able to decide but the choices are like those of a political election with only one candidate.
Over the last years, the notion of tooltips has somewhat changed.
Let me quote the doc string of the tooltip class for the original definition:

Provides information about an item over which the user is hovering a cursor.
The object should fire a show event when it appears.

MSDN gives a similar description, perhaps even narrower as they refer mainly to the mouse pointer.
However, one has to read between the lines to establish a different view point.
In contrast to the classic definition, tooltips can now appear outside the app.
In UIA terms, this would be "Peripheral", I think.
Let's call this new type flyouts.
Some classic tooltips are quite useful, e.g. those that you get in an explorer window with modification date, size etc. for a file.
Taskbar tooltips that are announced whilst the focus isn't there are rather a nuisance IMHO.
It is perhaps difficult to implement a finer distinction in NVDA core (e.g. by adding flyouts to
the object presentation dialog).

I think the best we can do is adding filter capability to the tooltip class such that add-ons can profit from it.

This is the perfect occasion to use the extensionPoints.Filter class.
After all, it has been introduced for exactly that purpose, has it not.

@josephsl

Copy link
Copy Markdown
Contributor Author

Hi,

Any additional comments?

The filter mechanism is useful for text processing, not really a way to let overlay classes change things here and there. To do that, one must be willing to add conditions in the class itself, or let global plugins and app modules take ownership of this class (inheritance and overriding).

Thanks.

@Robert-J-H

Robert-J-H commented Jun 17, 2018 via email

Copy link
Copy Markdown
Contributor

@gregjozk

gregjozk commented Aug 16, 2018

Copy link
Copy Markdown
Contributor

Hello, what is a status of this pr?

@josephsl

Copy link
Copy Markdown
Contributor Author

Hi,

Regarding deciders: this means extension points must be willing to deal with cases where tooltips are not announced because someone said "no" while the decider was running. I think this might not be what we want.

For now, I'd treat tooltips from UIA as just tooltips. If there are no major objections, I propose that this be ready for master branch soon.

Thanks.

@ehollig

ehollig commented Oct 13, 2018

Copy link
Copy Markdown
Contributor

Hello, What is the status of this PR? It looks like it has been incubating for a while now.

@josephsl

josephsl commented Oct 13, 2018 via email

Copy link
Copy Markdown
Contributor Author

@josephsl

Copy link
Copy Markdown
Contributor Author

Hi,

Actually, it is ready to go - I think it's just the matter of when it'll be included in master branch, and I can address review comments in October.

Thanks.

@dpy013

dpy013 commented Feb 2, 2019

Copy link
Copy Markdown
Contributor

Hello, what is the status of this PR now? I recently looked at the approved PRs and found that most of them did not merge. My idea is that I hope that the official can solve all the approved PRs and solve all the approved PRs, so that you can concentrate on developing and fixing problems and functions. The recommendations are for developers and maintainers only. Thank you

@LeonarddeR

Copy link
Copy Markdown
Collaborator

I removed the incubating label from this as we're no longer using that label. Having said that, the fact that this has been incubating for a while suggest that the code is of enough quality and has been approved in the past.

@LeonarddeR LeonarddeR requested a review from feerrenrut May 18, 2019 16:44
@josephsl

josephsl commented May 18, 2019 via email

Copy link
Copy Markdown
Contributor Author

@josephsl

Copy link
Copy Markdown
Contributor Author

Hi,

Let me merge in master first.

Thanks.

@josephsl

Copy link
Copy Markdown
Contributor Author

Hi,

Regarding review bits: yes, approved, but it won't hurt to let another pair of eyes/ears/hands take a look at this one before making this out to master.

Thanks.

@Robert-J-H

Robert-J-H commented May 18, 2019 via email

Copy link
Copy Markdown
Contributor

@dpy013

dpy013 commented May 21, 2019

Copy link
Copy Markdown
Contributor

This PR has the following error in the build:
Failure: AppVeyor build failed
Hope that can be fixed
thank
cc
@josephsl

@josephsl

josephsl commented May 21, 2019 via email

Copy link
Copy Markdown
Contributor Author

@AmadeusW

Copy link
Copy Markdown

How can I test changes in this PR? @josephsl do you have files I can use to patch NVDA 2019.1.1? I can install any other version, too, if that would make it easier for you to share the fix.

I'd like to verify whether this addresses issue where XAML tooltips are not announced in Visual Studio (VS is a Win32 app using WPF\XAML, not an Universal Windows app)

@josephsl

josephsl commented Jun 14, 2019 via email

Copy link
Copy Markdown
Contributor Author

@AmadeusW

Copy link
Copy Markdown

that's fine, I realized that NVDA does read some tooltips in VS, so this PR wouldn't have helped my case. Thank you!

@josephsl

Copy link
Copy Markdown
Contributor Author

Hi,

I'll make sure this PR passes checks.

Thanks.

josephsl added 3 commits June 25, 2019 08:35
Tooltips from universal apps are powered by XAML and has a specific class name (for now). Thus recognize them as proper tool tips.
…g ones from volume slider and other controls. Re nvaccess#8118.

Commented by Mick Curran (NV Access) and others: wrong assumption from my part - volume slider does not raise tool tip open event frequently, so this is safe to be called out. This is done by just checking for control type (same one as Windows 8.x toasts).
@feerrenrut feerrenrut merged commit 845a589 into nvaccess:master Jul 26, 2019
@nvaccessAuto nvaccessAuto added this to the 2019.3 milestone Jul 26, 2019
feerrenrut added a commit that referenced this pull request Jul 26, 2019
@josephsl

josephsl commented Jul 26, 2019 via email

Copy link
Copy Markdown
Contributor Author

@josephsl josephsl deleted the i8118uwpTooltips branch November 2, 2019 11:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Tool Tips are not read in UWP apps when "report tooltips" is enabled

10 participants