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 (2nd try) (nvaccess#17018)

Fixes nvaccess#14641
Addresses nvaccess#16995
Addresses nvaccess#16996

Summary of the issue:
In nvaccess#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 nvaccess#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.
  • Loading branch information
@XLTechie
XLTechie authored Aug 29, 2024
1 parent feebdec commit 811df33
Show file tree
Hide file tree
Showing 4 changed files with 203 additions and 46 deletions.
9 changes: 6 additions & 3 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 @@ -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
85 changes: 77 additions & 8 deletions source/message.html
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,24 +4,93 @@
<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
var copyEnabled = false;

function handleKeyPress(e) {
/*
This code should be effective for modern browsers (EdgeWebView2),
while also serving in old IE (MSHTML) contexts.
*/
e = e || window.event;
if(e.keyCode == 27){ // code for escape
window.close();
var key = e.key || e.keyCode;
if (key === 'Escape' || key === 27) { // code for escape
onCloseButtonPress();
} else if (e.shiftKey && e.ctrlKey && (key === 'C' || key === 67)) { // code for Ctrl+Shift+C
onCopyButtonPress();
}
}

function onCopyButtonPress() {
if (!copyEnabled) return;
// 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');
// We can't use innerText, even for text-only nodes,
// because of #12004.
messageDiv.innerHTML = 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) {
copyEnabled = true;
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>
151 changes: 117 additions & 34 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 All @@ -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"<pre>{escape(message)}</pre>"
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"<pre>{escape(message)}</pre>"
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(
Expand Down
4 changes: 3 additions & 1 deletion user_docs/en/changes.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand All @@ -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

Expand Down

0 comments on commit 811df33

Please sign in to comment.