From 811df33901267e609c1b404a1c9f521f52b3943c Mon Sep 17 00:00:00 2001 From: Luke Davis <8139760+XLTechie@users.noreply.github.com> Date: Thu, 29 Aug 2024 01:23:35 -0400 Subject: [PATCH] Give ui.browseableMessage the ability to show copy and close buttons after the message, and add buttons to some instances (2nd try) (#17018) Fixes #14641 Addresses #16995 Addresses #16996 Summary of the issue: In #14641 @Qchristensen reported that some users are confused by browseableMessages, and their lack of definite closure mechanisms. During the conversation, it was pointed out that in some cases, a user might also desire a copy button. During work on the PR (originally #16369), it was requested that the copy button be given an accelerator key. Description of user facing changes Added copy and close buttons to some browseableMessages, and the capability to add them to others. Description of development approach Thanks to @michaelDCurran's change in ui.browseableMessage--namely adding a Scripting.Dictionary to carry arbitrary query-string-equivalent style parameters to the mshtml instance behind browseableMessage, it is now possible to pass in values for two new buttons, and various translatable messages, without any contortions. Tried different methods in the JS and CSS of message.html, and eventually settled on one which displayed the two buttons side-by-side, under a separator. If neither button's label is supplied, the div containing the HR and buttons remains hidden. The "Copy" button can be activated with Ctrl+Shift+C, and the "Close" button by Escape. Alt+C is indicated to the user via an accessibility label. Fixed up some docstring parameter listings to match modern format. Per the issue, modified the Report Link Destination message, to have both a copy button and a close button. Per the issue, added buttons to the Report Character Information message. Per request in PR comments, used a live region to present a "Copy complete" or "copy failed" message to the user. This message remains on screen for five seconds. The AI suggested covering for more failures in the initialization and configuration of MSHTML. I used its suggested exception checking, but added a private function to display an error message to the user when one of the components fail, so at least some kind of user notice can be given. I based it on the already extant private function for messaging the user if a browseableMessage is called on a secure screen. Both of the warning functions for browseableMessage unavailable situations, are now self-contained, with respect to wx. Added an immediate return if the MSHTML window is opened without providing the dialog arguments (i.e. the message and title, at least). That situation should never actually occur, but in the unlikely event that a failure passes the error checking we have in browseableMessage, this will return immediately, instead of stranding the user in a blank window with no obvious close mechanism. --- source/globalCommands.py | 9 ++- source/message.html | 85 +++++++++++++++++++--- source/ui.py | 151 ++++++++++++++++++++++++++++++--------- user_docs/en/changes.md | 4 +- 4 files changed, 203 insertions(+), 46 deletions(-) diff --git a/source/globalCommands.py b/source/globalCommands.py index 6507ac4512d..8759807a22b 100755 --- a/source/globalCommands.py +++ b/source/globalCommands.py @@ -340,9 +340,8 @@ def script_reportCurrentSelection(self, gesture): if scriptCount == 0: speech.speakTextSelected(info.text) braille.handler.message(selectMessage) - elif scriptCount == 3: - ui.browseableMessage(info.text) + ui.browseableMessage(info.text, copyButton=True, closeButton=True) return elif len(info.text) < speech.speech.MAX_LENGTH_FOR_SELECTION_REPORTING: @@ -2441,6 +2440,8 @@ def _reportFormattingHelper(self, info, browseable=False): message, # Translators: title for formatting information dialog. _("Formatting"), + copyButton=True, + closeButton=True, ) @staticmethod @@ -4140,7 +4141,7 @@ def script_reportLinkDestination( ) -> None: """Generates a ui.message or ui.browseableMessage of a link's destination, if focus or caret is positioned on 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. + :param forceBrowseable: skips the press once check, and displays the browseableMessage version. """ try: ti: textInfos.TextInfo = api.getCaretPosition() @@ -4174,6 +4175,8 @@ def script_reportLinkDestination( # Translators: Informs the user that the window contains the destination of the # link with given title title=_("Destination of: {name}").format(name=obj.name), + closeButton=True, + copyButton=True, ) elif presses == 0: # One press ui.message(linkDestination) # Speak the link diff --git a/source/message.html b/source/message.html index 0e9396315e5..ada156164e7 100644 --- a/source/message.html +++ b/source/message.html @@ -4,24 +4,93 @@ - -
+ +
+ + + + + diff --git a/source/ui.py b/source/ui.py index 83bca0c26c8..c724d2486a8 100644 --- a/source/ui.py +++ b/source/ui.py @@ -46,10 +46,11 @@ HTMLDLG_VERIFY = 0x0100 -def _warnBrowsableMessageNotAvailableOnSecureScreens(title: Optional[str]) -> None: +def _warnBrowsableMessageNotAvailableOnSecureScreens(title: str | None = None) -> None: """Warn the user that a browsable message could not be shown on a secure screen (sign-on screen / UAC prompt). - @param title: If provided, the title of the browsable message to give the user more context. + + :param title: If provided, the title of the browsable message to give the user more context. """ log.warning( "While on secure screens browsable messages can not be used." @@ -72,68 +73,150 @@ def _warnBrowsableMessageNotAvailableOnSecureScreens(title: Optional[str]) -> No # The title may be something like "Formatting". "This feature ({title}) is unavailable while on secure screens" " such as the sign-on screen or UAC prompt.", - ) - browsableMessageUnavailableMsg = browsableMessageUnavailableMsg.format(title=title) + ).format(title=title) import wx # Late import to prevent circular dependency. import gui # Late import to prevent circular dependency. log.debug("Presenting browsable message unavailable warning.") - gui.messageBox( + wx.CallAfter( + gui.messageBox, + browsableMessageUnavailableMsg, + # Translators: This is the title for a warning dialog, shown if NVDA cannot open a browsable message. + caption=_("Feature unavailable."), + style=wx.ICON_ERROR | wx.OK, + ) + + +def _warnBrowsableMessageComponentFailure(title: str | None = None) -> None: + """Warn the user that a browsable message could not be shown because of a component failure. + + :param title: If provided, the title of the browsable message to give the user more context. + """ + log.warning( + "A browsable message could not be shown because of a component failure." + f" Attempted to open message with title: {title!r}", + ) + + if not title: + browsableMessageUnavailableMsg: str = _( + # Translators: This is the message for a warning shown if NVDA cannot open a browsable message window + # because of a component failure. + "An error has caused this feature to be unavailable at this time. " + "Restarting NVDA or Windows may solve this problem.", + ) + else: + browsableMessageUnavailableMsg: str = _( + # Translators: This is the message for a warning shown if NVDA cannot open a browsable message window + # because of a component failure. This prompt includes the title + # of the Window that could not be opened for context. + # The {title} will be replaced with the title. + # The title may be something like "Formatting". + "An error has caused this feature ({title}) to be unavailable at this time. " + "Restarting NVDA or Windows may solve this problem.", + ).format(title=title) + + log.debug("Presenting browsable message unavailable warning.") + import wx # Late import to prevent circular dependency. + import gui # Late import to prevent circular dependency. + + wx.CallAfter( + gui.messageBox, browsableMessageUnavailableMsg, - # Translators: This is the title for a warning dialog, shown if NVDA cannot open a browsable message - # dialog. + # Translators: This is the title for a warning dialog, shown if NVDA cannot open a browsable message. caption=_("Feature unavailable."), style=wx.ICON_ERROR | wx.OK, ) -def browseableMessage(message: str, title: Optional[str] = None, isHtml: bool = False) -> None: +def browseableMessage( + message: str, + title: str | None = None, + isHtml: bool = False, + closeButton: bool = False, + copyButton: bool = False, +) -> None: """Present a message to the user that can be read in browse mode. The message will be presented in an HTML document. - @param message: The message in either html or text. - @param title: The title for the message. - @param isHtml: Whether the message is html + + :param message: The message in either html or text. + :param title: The title for the message, defaults to "NVDA Message". + :param isHtml: Whether the message is html, defaults to False. + :param closeButton: Whether to include a "close" button, defaults to False. + :param copyButton: Whether to include a "copy" (to clipboard) button, defaults to False. """ if isRunningOnSecureDesktop(): - import wx # Late import to prevent circular dependency. - - wx.CallAfter(_warnBrowsableMessageNotAvailableOnSecureScreens, title) + _warnBrowsableMessageNotAvailableOnSecureScreens(title) return htmlFileName = os.path.join(globalVars.appDir, "message.html") if not os.path.isfile(htmlFileName): + _warnBrowsableMessageComponentFailure(title) raise LookupError(htmlFileName) + moniker = POINTER(IUnknown)() - windll.urlmon.CreateURLMonikerEx(0, htmlFileName, byref(moniker), URL_MK_UNIFORM) - if not title: - # Translators: The title for the dialog used to present general NVDA messages in browse mode. - title = _("NVDA Message") - if not isHtml: - message = f"
{escape(message)}
" + try: + windll.urlmon.CreateURLMonikerEx(0, htmlFileName, byref(moniker), URL_MK_UNIFORM) + except OSError as e: + log.error(f"OS error during URL moniker creation: {e}") + _warnBrowsableMessageComponentFailure(title) + return + except Exception as e: + log.error(f"Unexpected error during URL moniker creation: {e}") + _warnBrowsableMessageComponentFailure(title) + return + try: d = comtypes.client.CreateObject("Scripting.Dictionary") except (COMError, OSError): log.error("Scripting.Dictionary component unavailable", exc_info=True) - # Store the module level message function in a new variable since it is masked by a local variable with - # the same name - messageFunction = globals()["message"] - # Translators: reported when unable to display a browsable message. - messageFunction(_("Unable to display browseable message")) + _warnBrowsableMessageComponentFailure(title) return + + if title is None: + # Translators: The title for the dialog used to present general NVDA messages in browse mode. + title = _("NVDA Message") d.add("title", title) + + if not isHtml: + message = f"
{escape(message)}
" + else: + log.warning("Passing raw HTML to ui.browseableMessage!") d.add("message", message) + + # Translators: A notice to the user that a copy operation succeeded. + d.add("copySuccessfulAlertText", _("Text copied.")) + # Translators: A notice to the user that a copy operation failed. + d.add("copyFailedAlertText", _("Couldn't copy to clipboard.")) + if closeButton: + # Translators: The text of a button which closes the window. + d.add("closeButtonText", _("Close")) + if copyButton: + # Translators: The label of a button to copy the text of the window to the clipboard. + d.add("copyButtonText", _("Copy")) + # Translators: A portion of an accessibility label for the "Copy" button, + # describing the key to press to activate the button. Currently, this key may only be Ctrl+Shift+C. + # Translation makes sense here if the Control or Shift keys are called something else in a + # given language; or to set this to the empty string if that key combination is unavailable on some keyboard. + d.add("copyButtonAcceleratorAccessibilityLabel", _("control+shift+c")) + dialogArgsVar = automation.VARIANT(d) gui.mainFrame.prePopup() - windll.mshtml.ShowHTMLDialogEx( - gui.mainFrame.Handle, - moniker, - HTMLDLG_MODELESS, - byref(dialogArgsVar), - DIALOG_OPTIONS, - None, - ) - gui.mainFrame.postPopup() + try: + windll.mshtml.ShowHTMLDialogEx( + gui.mainFrame.Handle, + moniker, + HTMLDLG_MODELESS, + byref(dialogArgsVar), + DIALOG_OPTIONS, + None, + ) + except Exception as e: + log.error(f"Failed to show HTML dialog: {e}") + _warnBrowsableMessageComponentFailure(title) + return + finally: + gui.mainFrame.postPopup() def message( diff --git a/user_docs/en/changes.md b/user_docs/en/changes.md index 07ff3e8d5e4..fa9153d6352 100644 --- a/user_docs/en/changes.md +++ b/user_docs/en/changes.md @@ -10,7 +10,8 @@ ### Changes -* The exit dialog now allows you to restart NVDA with add-ons disabled and debug logging enabled simultaneously. (#11538, @CyrilleB79) +* The Report link destination, Character formatting information, and Speak selection dialogs, now include "Close" and "Copy" buttons for user convenience. (#17018, @XLTechie) +* The exit dialog now allows you to restart NVDA with add-ons disabled and debug logging enabled simultaneously. (#11538, @CyrilleB79)r ### Bug Fixes @@ -22,6 +23,7 @@ Please refer to [the developer guide](https://www.nvaccess.org/files/nvda/docume * Note: this is an Add-on API compatibility breaking release. Add-ons will need to be re-tested and have their manifest updated. +* `ui.browseableMessage` may now be called with options to present a button for copying to clipboard, and/or a button for closing the window. (#17018, @XLTechie) #### API Breaking Changes