Uniformize all "Copied to clipboard" messages (#6757) by JulienCochuyt · Pull Request #9843 · nvaccess/nvda

JulienCochuyt · 2019-06-28T15:05:34Z

Link to issue number:

Summary of the issue:

The 5 clipboard-related scripts do not uniformly emit acknowledgement messages.
They present success either with speech&braille or speech only.
They either report failure, or not.
Three of them are triple-press gestures. The possibly lengthy copied text being reported before the statement "copied to clipboard" does not help to make it clear something different occurred.

Description of how this pull request fixes the issue:

Centralize reporting of success or failure in one place (inspired by Move the message 'selected' after the text which is selected. #9028)
Always report failure, reusing the existing "Unable to copy"
Announce "Copied to clipboard: {text}"
Display in braille the shorter "Copied: {text}" to accommodate smaller displays
Client code can benefit from the new notify parameter to api.copyToClip to obtain the new standard behavior. This new parameter defaults to False not to interfere with existing calls from addons.

Testing performed:

Tested with both speech and braille:

Copy the current navigator object
Copy the application title bar
Copy the application status bar
Copy the review selection (NVDA+F10)
Copy the cursor manager selection (control+C)

Known issues with pull request:

This PR adds the first translated message to the api module.
If this is to be considered a problem, the message part could easily be moved out to something like ui.acknowledgeCopyToClipboard(text: str, success: bool)

Change log entry:

Section: Changes
NVDA will now report "Copied to clipboard" before the copied text.

JulienCochuyt · 2019-07-15T14:13:55Z

72e9d14: Rebased onto latest master

JulienCochuyt · 2019-08-13T12:02:44Z

Rebased onto latest master and linted.

lukaszgo1 · 2019-11-25T22:55:55Z

While it is certainly too late to include this into 2019.3 it would be great if it can be considered for the next release. cc @feerrenrut @michaelDCurran

feerrenrut · 2020-04-08T12:21:18Z

Does this require updates to the user guide?

lukaszgo1 · 2020-04-28T16:30:16Z

@feerrenrut wrote:

Does this require updates to the user guide?

IMHO it does not @JulienCochuyt Do you agree? I'd be great to push this forward - the change is quite small and in my opinion PR's such as this one i.e cosmetic changes are contributing to the backlog greatly.

JulienCochuyt · 2020-04-30T08:44:23Z

@feerrenrut wrote:
Does this require updates to the user guide?

I couldn't find in the user guide any mention of the exact announce performed when text is copied.
Thus, no, I don't think any impact is needed there.

@lukaszgo1 wrote:

IMHO it does not @JulienCochuyt Do you agree? I'd be great to push this forward - the change is quite small and in my opinion PR's such as this one i.e cosmetic changes are contributing to the backlog greatly.

Thanks for your support.

feerrenrut · 2020-04-30T13:07:12Z

+			# Translators: Announced when a text has been copied to clipboard.
+			# %s is replaced by the copied text.
+			speech.speakMessage(_("Copied to clipboard: %s" % text))
+			# Translators: Displayed in braille when a text has been copied to clipboard.
+			# %s is replaced by the copied text.
+			braille.handler.message(_("Copied: %s" % text))


I'd prefer not import speech for this. Instead, I think we should add an optional "brailleMessage" argument to ui.message. That way, the detail for how messaging is done is abstracted away.
Actually, I think this is only place in api.py that uses the ui module. We have long had requests for greater customization of UI messages collecting this UI together will probably help (also with consistency of messages). Perhaps the messages should be moved into the ui module, EG: def reportTextCopiedToClipboard(text: Optional[str]) -> None. When text is None, "Unable to copy" is reported. This can be called from each place that wishes to notify, rather than doing it in api.

Thinking about why these are different, perhaps both speech and braille should simply use "Copied: %s"? It's shorter, and arguably "to clipboard" doesn't tell the user anything.

I just rebased onto latest master.
Done adding a brailleText parameter to ui.message.
Done moving "Unable to copy" within ui.reportTextCopiedToClipboard.

Regarding the lengthier spoken message, I felt it was closer to what is currently announced eg. in browse mode.
Let me know if you wish I change this anyway.

feerrenrut · 2020-04-30T13:26:16Z

I couldn't find in the user guide any mention of the exact announce performed when text is copied. Thus, no, I don't think any impact is needed there.

@JulienCochuyt Thanks for checking.

Review action: nvaccess#9843 (comment)

Re: nvaccess#9844 (comment)

codeofdusk · 2020-08-27T11:11:55Z

@feerrenrut Any updates on this PR?

lukaszgo1 · 2020-11-15T21:57:46Z

It seems all requested changes are addressed on this one. @feerrenrut Could it be that this got of your radar?

Adriani90 · 2020-11-20T23:53:51Z

Not sure if it comes from this PR, but I get now following error in NVDA alpha Version: alpha-21386,53cecfd7

STR:

Open MS Outlook
Open an Email
Enable browse mode and select some text
Press ctrl+c

Actual: Following error is raised and the selected text is not copied to the clipboard:

IO - inputCore.InputManager.executeGesture (00:52:29.103) - winInputHook (1524):
Input: kb(laptop):control+c
ERROR - scriptHandler.executeScript (00:52:29.113) - MainThread (8596):
error executing script: <bound method CursorManager.script_copyToClipboard of <appModules.outlook.MailViewerTreeInterceptor object at 0x00FB6530>> with gesture 'Strg+c'
Traceback (most recent call last):
  File "scriptHandler.pyc", line 208, in executeScript
  File "cursorManager.pyc", line 409, in script_copyToClipboard
  File "treeInterceptorHandler.pyc", line 193, in copyToClipboard
TypeError: copyToClipboard() takes 1 positional argument but 2 were given

Expected: No error and the selected text is copied to the clipboard.

JulienCochuyt · 2020-11-22T13:26:20Z

@Adriani90 wrote:

Not sure if it comes from this PR, but I get now following error in NVDA alpha Version: alpha-21386,53cecfd7

It does. My bad. Thank you for the report and STR.

…ation from being reported.

Fix-up of #9843

JulienCochuyt mentioned this pull request Jun 30, 2019

Option to disable the announcement "Copied to clipboard" #9273

Open

JulienCochuyt force-pushed the i6757 branch from 5258c84 to 72e9d14 Compare July 15, 2019 14:13

JulienCochuyt force-pushed the i6757 branch from 72e9d14 to 2a9bdb2 Compare August 13, 2019 12:02

feerrenrut added the enhancement label Apr 8, 2020

feerrenrut suggested changes Apr 30, 2020

View reviewed changes

JulienCochuyt added a commit to accessolutions/nvda that referenced this pull request May 1, 2020

Do not bind api to speech / Move message literals to ui

b201df8

Review action: nvaccess#9843 (comment)

feerrenrut self-requested a review May 5, 2020 16:08

JulienCochuyt added 4 commits July 2, 2020 16:25

Uniformize all "Copied to clipboard" messages (nvaccess#6757)

50bf96b

Do not bind api to speech / Move message literals to ui

8f7ca6f

Review action: nvaccess#9843 (comment)

Declare encoding, as an alternative to PR nvaccess#11081

2bd0c90

Update copyright notice header lines and split instead of disabling E501

1da6d14

Re: nvaccess#9844 (comment)

JulienCochuyt force-pushed the i6757 branch from 0cf56dc to 1da6d14 Compare July 2, 2020 14:29

feerrenrut previously approved these changes Nov 17, 2020

View reviewed changes

feerrenrut added 2 commits November 17, 2020 15:32

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

1b112a1

update changes file for PR nvaccess#9843

b357c39

feerrenrut dismissed their stale review via b357c39 November 17, 2020 07:35

feerrenrut approved these changes Nov 17, 2020

View reviewed changes

feerrenrut merged commit 318c215 into nvaccess:master Nov 17, 2020

nvaccessAuto added this to the 2020.4 milestone Nov 17, 2020

JulienCochuyt deleted the i6757 branch November 17, 2020 08:13

CyrilleB79 mentioned this pull request Nov 18, 2020

Unable to copy text from MS Word virtual buffer #11839

Closed

CyrilleB79 mentioned this pull request Nov 18, 2020

NVDA freeze when copying large amount of text #11843

Closed

lukaszgo1 mentioned this pull request Nov 20, 2020

Fix up of PR 9843: Once again report when copying text in browse mode in MS Word. #11848

Merged

JulienCochuyt added a commit to accessolutions/nvda that referenced this pull request Nov 22, 2020

Fix-up of PR nvaccess#9843: Impact MS Word browse mode

f825d02

JulienCochuyt mentioned this pull request Nov 22, 2020

Fix-up of PR #9843: Impact MS Word browse mode #11851

Closed

JulienCochuyt mentioned this pull request Nov 22, 2020

Fix NVDA may freeze when copying large amount of text (#11843) #11852

Merged

CyrilleB79 mentioned this pull request Dec 21, 2020

Fix-up of #9843: Fixed an error preventing a failed copy operation from being reported [NVDA 2020.4beta1] #11959

Merged

CyrilleB79 added a commit to CyrilleB79/nvda that referenced this pull request Dec 21, 2020

Fix-up of nvaccess#9843: Fixed an error preventing a failed copy oper…

546e711

…ation from being reported.

feerrenrut pushed a commit that referenced this pull request Dec 22, 2020

Fix error preventing a report of failed copy operation (PR #11959)

4a451d0

Fix-up of #9843

Adriani90 mentioned this pull request Apr 10, 2024

NVDA does not announce Copied to clipboard in windows mail #16375

Open

Uh oh!

Conversation

JulienCochuyt commented Jun 28, 2019 • edited 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 how this pull request fixes the issue:

Testing performed:

Known issues with pull request:

Change log entry:

Uh oh!

JulienCochuyt commented Jul 15, 2019

Uh oh!

JulienCochuyt commented Aug 13, 2019

Uh oh!

lukaszgo1 commented Nov 25, 2019

Uh oh!

feerrenrut commented Apr 8, 2020

Uh oh!

lukaszgo1 commented Apr 28, 2020

Uh oh!

JulienCochuyt commented Apr 30, 2020

Uh oh!

feerrenrut Apr 30, 2020

Choose a reason for hiding this comment

Uh oh!

JulienCochuyt Jul 2, 2020

Choose a reason for hiding this comment

Uh oh!

feerrenrut commented Apr 30, 2020

Uh oh!

codeofdusk commented Aug 27, 2020

Uh oh!

lukaszgo1 commented Nov 15, 2020

Uh oh!

Adriani90 commented Nov 20, 2020

Uh oh!

JulienCochuyt commented Nov 22, 2020

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

JulienCochuyt commented Jun 28, 2019 •

edited

Loading