Skip to content

Commit

Permalink
Give ui.browseableMessage the ability to show copy and close buttons …
Browse files Browse the repository at this point in the history 
…after the message, and add buttons to some instances (#16369)

Fixes #14641

Summary of the issue:
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.

Description of user facing changes
Added copy and close buttons to some browseableMessages.

Description of development approach
Thanks to @michaelDCurran's recent browseableMessage change--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 Alt+C, and the "Close" button by Escape. Alt+C is indicated to the user via an accessibility label.
While the description of the key has been made translatable, the key itself can not be changed currently.
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 a close button 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.
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.
Tightened the handling of text vs. HTML, to possibly prevent some hypothetical cases of raw HTML being given to the browser instance when we don't expect that to be the case. In particular, messages are now always assumed to be text by the javascript processing, unless a specific flag is passed via the scriptable.dictionary, instructing the JS to allow the message to be treated as HTML.
  • Loading branch information
@XLTechie
XLTechie authored Aug 12, 2024
1 parent fa5cf4f commit 2e09be1
Show file tree
Hide file tree
Showing 4 changed files with 190 additions and 36 deletions.
11 changes: 7 additions & 4 deletions source/globalCommands.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down Expand Up @@ -2210,7 +2209,7 @@ def script_review_currentSymbol(self, gesture):
languageDescription = languageHandler.getLanguageDescription(curLanguage)
# Translators: title for expanded symbol dialog. Example: "Expanded symbol (English)"
title = _("Expanded symbol ({})").format(languageDescription)
ui.browseableMessage(message, title)
ui.browseableMessage(message, title, closeButton=True)

@script(
description=_(
Expand Down Expand Up @@ -2441,6 +2440,8 @@ def _reportFormattingHelper(self, info, browseable=False):
message,
# Translators: title for formatting information dialog.
_("Formatting"),
copyButton=True,
closeButton=True,
)

@staticmethod
Expand Down Expand Up @@ -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()
Expand Down Expand Up @@ -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
Expand Down
78 changes: 72 additions & 6 deletions source/message.html
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,24 +4,90 @@
<TITLE></TITLE>
<SCRIPT LANGUAGE=javascript>
<!--
function escapeKeyPress(e) {
// Though these should be constants, MSHTML does not support const as of 2024
var args = window.dialogArguments;
var copyStatusTimeout = 5000; // milliseconds before copy status message goes away

function handleKeyPress(e) {
e = e || window.event;
if(e.keyCode == 27){ // code for escape
window.close();
} else if (e.altKey && e.keyCode == 67) { // code for Alt+C
onCopyButtonPress();
}
}

function onCopyButtonPress() {
// Copy code came from http://www.codestore.net/store.nsf/unid/DOMM-4QHQE8/
var rng = document.body.createTextRange();
rng.moveToElementText(messageDiv);
rng.scrollIntoView();
rng.select();
var success = rng.execCommand("Copy");
rng.collapse(false);
rng.select();
copyStatusDiv.innerHTML = '<p id="copyStatusText" style="font-weight:bold; text-align:center;"></p>';
if (success) { // Notify the user about the copy result
copyStatusText.innerText = args.item('copySuccessfulAlertText');
} else {
copyStatusText.innerText = args.item('copyFailedAlertText');
}
};
// Time out the user alert message
setTimeout(function() {
copyStatusDiv.innerHTML = '';
}, copyStatusTimeout);
}

function onCloseButtonPress() {
window.close();
}

function windowOnLoad() {
var args = window.dialogArguments;
if (args) {
document.title = args.item('title');
messageID.innerHTML = args.item('message');
// Decide if we are displaying a text only (escaped, sanitized) message, or
// if the caller has sent a raw HTML message that must be interpreted by the browser
if (args.item('messageUsesRawHTML') != null) { // Was set, assume HTML
messageDiv.innerHTML = args.item('message');
} else { // A text only message; the majority of usages
messageDiv.innerHTML = '<pre id="textOnlyMessage"></pre>';
textOnlyMessage.innerText = args.item('message');
}
// If caller wants a close button
if (args.item('closeButtonText') != null) {
closeButton.innerText = args.item('closeButtonText'); // Assign the (translated) label
closeButton.style.display = 'inline'; // Display the button
buttonDiv.style.display = 'block'; // Display the button section
}
// If caller wants a copy to clip button
if (args.item('copyButtonText') != null) {
copyButton.innerText = args.item('copyButtonText'); // Assign the (translated) label
copyButtonSpan.style.display = 'inline'; // Display the button
copyStatusDiv.style.display = 'block'; // Display the copy status
buttonDiv.style.display = 'block'; // Display the section
if (args.item('copyButtonAcceleratorAccessibilityLabel') != null) { // If translated text for the accelerator is provided
copyButtonAcceleratorAccessibilityLabel.innerText = args.item('copyButtonAcceleratorAccessibilityLabel');
}
}
} else { // If args wasn't provided, we can do nothing. Just exit
window.close();
}
}
//-->
</SCRIPT>
</HEAD>
<BODY tabindex='0' id='main_body' style="margin:1em" LANGUAGE=javascript onload="return windowOnLoad()" onkeypress="return escapeKeyPress()" >
<div id=messageID></div>
<body tabindex='0' id='main_body' style="margin:1em" onload="return windowOnLoad()" onkeydown="return handleKeyPress()" >
<div id="messageDiv"></div>

<!-- "tabindex" is needed to circumvent an old IE/MSHTML bug in the handling of aria-labelledby.
See: https://www.tpgi.com/aria-labelledby-aria-describedby-support-popular-windows-browsers/ -->
<span id="copyButtonAcceleratorAccessibilityLabel" style="visibility: hidden;" tabindex="-1"></span>

<div id="buttonDiv" style="display: none;"><hr>
<div id="copyStatusDiv" role="status" aria-live="polite"></div>
<span id="copyButtonSpan" style="display: none;">
<button id="copyButton" onclick="onCopyButtonPress();" aria-labelledby="copyButton copyButtonAcceleratorAccessibilityLabel"></button>&nbsp;</span>
<span><button id="closeButton" style="display: none;" onclick="onCloseButtonPress();"></button></span>
</div>
</body>
</html>
135 changes: 109 additions & 26 deletions source/ui.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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."
Expand Down Expand Up @@ -88,13 +89,67 @@ def _warnBrowsableMessageNotAvailableOnSecureScreens(title: Optional[str]) -> No
)


def browseableMessage(message: str, title: Optional[str] = None, isHtml: bool = False) -> None:
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.",
)
browsableMessageUnavailableMsg = browsableMessageUnavailableMsg.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(
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 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 title is None:
# Translators: The title for the dialog used to present general NVDA messages in browse mode.
title = _("NVDA Message")

if isRunningOnSecureDesktop():
import wx # Late import to prevent circular dependency.

Expand All @@ -103,37 +158,65 @@ def browseableMessage(message: str, title: Optional[str] = None, isHtml: bool =

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"<pre>{escape(message)}</pre>"
try:
windll.urlmon.CreateURLMonikerEx(0, htmlFileName, byref(moniker), URL_MK_UNIFORM)
except Exception as e:
log.error(f"Failed to create URL moniker: {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
d.add("title", title)

if not isHtml:
message = escape(message)
else:
d.add("messageUsesRawHTML", "true") # Value doesn't matter, as long as there is one
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 Alt+C.
# Translation makes sense here if the Alt key is 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", _("Alt+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(
Expand Down
2 changes: 2 additions & 0 deletions user_docs/en/changes.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ The available options are:
### Changes

* The `-c`/`--config-path` and `--disable-addons` command line options are now respected when launching an update from within NVDA. (#16937)
* The Report link destination, Character formatting information, and Speak selection dialogs, now include "Close" and "Copy" buttons for user convenience. (#16369, @XLTechie)
* eSpeak NG has been updated to 1.52-dev commit `961454ff`. (#16775)
* Added new languages Faroese and Xextan.

Expand Down Expand Up @@ -54,6 +55,7 @@ Please refer to [the developer guide](https://www.nvaccess.org/files/nvda/docume
* Please consult the [Custom speech symbol dictionaries section in the developer guide](https://www.nvaccess.org/files/nvda/documentation/developerGuide.html#AddonSymbolDictionaries) for more details.
* It is now possible to redirect objects retrieved from on-screen coordinates, by using the `NVDAObject.objectFromPointRedirect` method. (#16788, @Emil-18)
* Running SCons with the parameter `--all-cores` will automatically pick the maximum number of available CPU cores. (#16943, #16868, @LeonarddeR)
* `ui.browseableMessage` may now be called with options to present a button for copying to clipboard, and/or a button for closing the window. (#16369, @XLTechie)
* Developer info now includes information on app architecture (such as AMD64) for the navigator object. (#16488, @josephsl)

#### Deprecations
Expand Down

0 comments on commit 2e09be1

Please sign in to comment.