Added global commands which allow a user to easily learn the destination of a link#14583
Conversation
|
Hi, this will interfere with Place Markers add-on by @nvdaes. Thanks.
|
See test results for failed build of commit 70f1c2ca10 |
CyrilleB79
left a comment
There was a problem hiding this comment.
LGTM globally. Just providing some minor suggestions.
Also, I think that this script is needed by a lot of people. Thus, I support the fact that you have attributed a gesture to the script (instead of leaving it unassigned). I think that there is also an equivalent script in recent versions of Jaws.
| gesture="kb:NVDA+k", | ||
| category=SCRCAT_TOOLS | ||
| ) | ||
| def script_reportLinkDestination(self, gesture, forceBrowseable: bool = False) -> None: |
There was a problem hiding this comment.
Since you are using type hints (which is a good thing), please also add it for the parameter gesture.
| """Generates a ui.message or ui.browseableMessage of a link's destination, if the navigator | ||
| object is a link, or an element with an included link such as a graphic. | ||
| @param forceBrowseable: skips the press once check, and displays the browseableMessage version. | ||
| @type forceBrowseable: bool |
There was a problem hiding this comment.
Please remove this line since there is already the type hint in the signature of the function.
| @type forceBrowseable: bool |
| ), | ||
| category=SCRCAT_TOOLS | ||
| ) | ||
| def script_reportLinkDestinationInWindow(self, gesture) -> None: |
| ): | ||
| if ( | ||
| forceBrowseable # If a browseable message is preferred unconditionally, or | ||
| or scriptHandler.getLastScriptRepeatCount() > 0 # if pressed more than once |
There was a problem hiding this comment.
IMO it does not make sense to press the gesture more than twice. Thus, could you rather make the script so that pressing the gesture three times or more does not produce any further effect?
There was a problem hiding this comment.
@CyrilleB79 I am unclear if your comment relates to logic or behavior.
I wrote the code like that, so we could do the test for forceBrowseable, before running the pressed more than once check. That way, the check for number of presses doesn't get run if it isn't needed.
To do that with as little if-thening as possible, I had to check the two-press state before the one-press state.
If you would find it more correct, I can change the "> 0" to "== 1", which aught to produce the same effect, but would leave the three-press case undefined.
As it stands, it doesn't react to three presses. That is, if you press three times, it has already opened the browseableMessage, and the third press is registered as a new first press, which says "not a link". Is that not the expected behavior?
Your other comments are fixed in latest commit - thanks.
There was a problem hiding this comment.
As it stands, it doesn't react to three presses. That is, if you press three times, it has already opened the browseableMessage, and the third press is registered as a new first press, which says "not a link". Is that not the expected behavior?
Yes, it is the expected behaviour. However on my system, when I triple-press, I get 2 browseable messages opened. Probably at the moment I perform the third press, the focus has not yet changed.
|
Hi, one possible justification for assigning NVDA+D instead: 1800048 - Fetching IA2/ATK relations on links in the WHATWG HTML spec is slow (mozilla.org) <https://bugzilla.mozilla.org/show_bug.cgi?id=1800048>
If links are treated as annotations (some might argue that), then I think treating hyperlink ref would be a good use of NVDA+D unless people think otherwise. NVDA+K makes sense from consistency perspective. Thanks.
|
See test results for failed build of commit fd8d439338 |
|
Hi, one possible justification for assigning NVDA+D instead
I may have missed something, if the existing script on NVDA+d is going to be
removed, I suppose we could.
|
See test results for failed build of commit b34d14aa40 |
|
I think the discussion about key commands is rather one about prioritization. In my opinion the key commands that are becomming the core of NVDA should be in favor compared to addons. Thus, actually the commands of the addons could be changed instead. This would also bring the benefit of the addon developers being incentivized to update their addons in order to work properly. |
|
Thanks for letting me know about possible conflict with placeMarkers. In case it"s needed, I"ll use a different command in the add-on. |
|
Thank you @nvdaes. Would @Qchristensen or @seanbudd care to comment on whether NVDA+k is the preferred key for this, or if something else would be more desirable? |
|
Hello,
Please keep in mind that NVDA+K is a conflicting command for Braille Extender's Reload Driver function. Consider an alternate for either this proposed change or, if one of the maintainers for it, the mentioned add-on.
Thanks.
Chris
On Jan 25, 2023, at 21:18, Luke Davis ***@***.***> wrote:
Thank you @nvdaes <https://github.com/nvdaes>. Would @Qchristensen <https://github.com/Qchristensen> or @seanbudd <https://github.com/seanbudd> care to comment on whether NVDA+k is the preferred key for this, or if something else would be more desirable?
—
Reply to this email directly, view it on GitHub <#14583 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/APKCRXYJLEJ3IDWZ3E2XUXDWUHUHXANCNFSM6AAAAAAUGEAQEQ>.
You are receiving this because you are subscribed to this thread.
|
|
@CyrilleB79 I was only able once to replicate the behavior you described. Try this next build when it's done please, and let me know if the double window problem persists. Thanks for your testing. |
The double window problem is not present anymore. The logic of the code remains a bit strange, i.e. for a triple press, the function reports again the URL instead of doing nothing. However, from the UX point of view, it has no impact since the reported URL is not heard due to the browseable message opening and generating a focus event. You may want to do the code clearer, replacing the " |
|
@CyrilleB79 wrote:
The logic of the code remains a bit strange, i.e. for a triple press, the function reports again the URL instead of doing nothing. However, from the UX
point of view, it has no impact since the reported URL is not heard due to the browseable message opening and generating a focus event. You may want to do
the code clearer, replacing the "else" by: "elif scriptHandler.getLastScriptRepeatCount() == 0:"
Not how I did it, but I believe I have achieved what you want.
Check latest build (running now).
As an aside: you started to ask something to native English speakers, in one of
your PR comments. Your question appeared cut-off. I tried to email you to
continue that question, but your email system rejected my message, does not like
my mail provider for some reason.
|
The code is clearer now, thanks. And of course it is still working correctly.
It was a piece of message left over unintentionally. I had then edited my message to remove it. I was just concerned by the spelling of "browseable" but after some research, it seems that "browseable" is just an alternate valid spelling for "browsable". |
|
@CyrilleB79 wrote:
It was a piece of message left over unintentionally. I had then edited my message to remove it. I was just concerned by the spelling of "browseable" but
after some research, it seems that "browseable" is just an alternate valid spelling for "browsable".
Apparently. I went through the same thing when I first found the function
ui.browseableMessage, some years ago. That's not how we would do it in U.S.
English.
Therefore while I have never liked that spelling, evidently that's how we spell
it in NVDA. That's the only reason I have spelled it that way here--I guess it
is preferred . . .
|
…ion of a link. (#14583) * Two new commands have been added: - NVDA+k (all layouts): Report the destination URL of the link in the navigator object Pressed once, will speak/braille the link, or other element containing a link, represented by the navigator object. Pressed twice, will show the link text in a browseable message, with the link name as its title. - Unassigned: Report the destination URL of the link in the navigator object in a window Will skip directly to showing the browseable message, as above. This second option is imagined as an alternative to which braille users might prefer to remap NVDA+k. The NVDA+k key was chosen because of its relatability to the browse mode K command. * Updated the copyright of globalCommands.py * Updated the "Reporting location and other information" section of the user guide.
|
Re the keystroke. My first thought was NVDA+d, thinking of the old "long description" use. Now that is used for aria-describedby, it could do double duty, although we'd need to work out what would happen if something had both aria-describedby AND a link. Otherwise, NVDA+k is an option, although as noted, that conflicts with several add-ons. Increasingly I think it will be hard to find new NVDA+one key commands which don't conflict with add-ons. So, in short, I don't have a firm "It absolutely must be this keystroke" opinion, but just to throw one more in to the mix: NVDA+numpad delete / NVDA+delete. "Reports information about the location of the text or object at the position of system caret. " Currently, it tells you the location on screen, which seems like a very rare use case, compared to wanting to know where a link will take you. |
|
My preference would probably be NVDA+k. It relates to k (link) in quicknav.
I do acknowledge that this gesture is already taken by certain add-ons,
but in that case, the add-on should win anyway - NVDA won't steal the
existing gesture from the add-on, though does depend on exactly how the
add-on was implemented. However, the add-on author / user could change
the gesture if they wanted to get to this new script.
Putting it in to either NvDA+d or NVDA+numpadDelete is possible, but
could complicate things as to when and when not certain information is
reported.
|
|
I still agree with nvda +K, @nvdaes expressed willingness to change this gesture in PlaceMarkers, and other add-ons that may exist may be requested by users to change the gesture |
Qchristensen
left a comment
There was a problem hiding this comment.
Looks good and will be very useful, thanks!
|
Just trying this alpha build, it all works great. When you press the keystroke twice, the window displayed is nearly the height of the screen, and about half the width of the screen. The URL - which on the link I tested (the download link for the alpha build) is wider than the window - continues off the right edge of the window. The window has a scrollbar to accommodate this, however it seems odd to have such a high window when the link, which doesn't wrap, can only ever be one line high. It might be worth having the link wrap so that it is all visible (and ideally having the window fit to size so everything is neatly displayed on screen in a window which isn't overly large for the text in it. Screenshot of how it appears on my system: And for those it helps, here is a (very rough) mockup of how I'd maybe have the window resized: https://www.dropbox.com/s/63h0rlcikb3fx4m/links2.jpg?dl=0 (Happy to write up a new issue if that is useful - the functionality as it stands is very much more useful than not having it, so I would not propose reverting it, I'm just suggesting this as an improvement for the dialog when the keystroke is pressed twice). |
|
It might be worth having the link wrap so that it is all visible (and ideally having the window fit to size so everything is
neatly displayed on screen in a window which isn't overly large for the text in it.
I agree with the second point--I foolishly didn't have anyone sighted review
this, mistakenly assuming that ui.browseableMessage had common sense auto
fitting.
I'll see what can be done in a followup PR.
Regarding your first point, however, I would hesitate to do a line break,
because it would inhibit the user's ability to select all and copy the link, if
desired.
While that is not the purpose of this feature, and the browser's copy
destination feature should be what is used, we can't prevent that usage, and so
it should behave as the user expects.
The best I *think* we can do, is to have it stylistically break every X
characters (40?), but not break when selected and copied.
Something like described here:
https://stackoverflow.com/questions/28376684/breaking-long-strings-visually-into-chunks-using-css-but-retaining-original-for
That will not resize with the Window, so presumably the window could be shrunk
to the point where it would again be more narrow than the URL, but it might
solve the immediate "bad look" problem.
Thoughts?
|
|
I'm just trying to think of an equivalent dialog - the speech viewer dialog works how I had in mind - text wraps as needed to fit within the window - but if you copy the text to say Notepad, the text only breaks at a proper line break (or end of an utterance at least) - not where it wrapped in the speech viewer. I agree re the need to keep the link whole for the purpose of copying it - while it is not necessarily the most direct way to copy the link originally, if you've opened this window to review it, the next logical action may be to want to open the URL if you are happy, and one common way to do that from here would likely be to copy it in order to open it in a new browser window |
…rowseableMessage, for less ambiguity when separating title from message. (#14668) Fixes #14667 Summary of the issue: If ui.browseableMessage was given a title containing a semicolon (;), the title would be broken at the semicolon, and the rest of the title would be rendered before (as part of) the message in the HTML portion of the window. This had the potential to result in unexpected outcomes, if the message content was HTML; not to mention being confusing for the user. While this was noticed in relation to the Report link destination feature introduced by #14583, it did not result from that feature, and could have occurred prior to its introduction. It does however effect it significantly, as the title of the link the destination of which is being identified, is presented in the title of a ui.browseableMessage, if the command is called twice, or the alternate version of the command is mapped to a key. Description of user facing changes If a link name contains a semicolon, portions of that name will no longer appear along with the link address, when reporting link destination in its window version. Description of development approach Changed the regex separator string in ui.browseableMessage, and in the message.html file, to one which should hopefully be much less likely to appear in the wild. Instead of a single semicolon (";"), the string is now "__NVDA:split-here__". Also removed the blank line from the top of message.html. I know of no purpose for having a blank line there.
|
Note: the Outlook test above no longer passes. At least in Outlook 2019, I can not cause links to be read in either browse or focus mode, while either composing or reading. |
…object (#14723) Closes #14659 Summary of the issue: In #14583 @XLTechie added a global commands which allows to report a URL for the link at the position of the navigator object. While this is a huge improvement compared to either having copy the URL to the clipboard or accessing the URL from the status bar from the web browser, the fact that these scripts report destination for the link at the navigator object rather than focus / caret position makes their usage impractical for people who prefer to work with review cursor / navigator object not following focus / caret. Description of user facing changes When user requests URL of a link NVDA reports URL of a link at the position of the caret / focus. Description of development approach Script for reporting URL was modified so that it first tries to get an object at the current caret position, and if that fails from the focused object. User guide and documentation for global commands were updated accordingly. After @XLTechie's review it was noticed that the script fails to work for links without href attribute, as part of this PR I've added handling for such links.
Link to issue number:
Closes #14535
Summary of the issue:
@Qchristensen pointed out, that in this age of phishing and other daily digital security threats effecting average users across the globe, the ability to determine where a link on the web, in an email, or anywhere else, is actually going to take you, is of increasing importance.
NVDA has no current way to do that in most/all circumstances.
Description of user facing changes
Added the following global commands:
The NVDA+k key was chosen because of its relatability to the browse mode K command.
I also added the
NVDA+kkey and description, to the "Reporting location and other information" section of the user guide. I did not add information about the unmapped command, or a broader description of the feature in general, because I was not sure of an appropriate section for it, though it may be worth adding a "Miscellaneous Features" section at some point.Description of development approach
globalCommands.py. The first will output the link, if the navigator object is or contains a link (such as a graphic with a link), as aui.messageif called once, or as aui.browseableMessageif called twice.ui.browseableMessage.Testing strategy:
Known issues with pull request:
I can only make this work in MS Word browse mode, if I set UIA to always.
In focus mode, neither UIA or DOM indicate links properly.
While I consider this an issue to be solved, I think the fact that the current implementation works in most other places, warrants submitting the PR now.
I am not equipped to test this with braille or for low vision.
Change log entries:
New features
Code Review Checklist: