displaying font information in a virtual message will be helpful. #4908

Closed
nvaccessAuto opened this Issue Feb 12, 2015 · 36 comments

Comments

Projects
None yet
8 participants

Reported by sumandogra on 2015-02-12 07:16
NVDA needs to have A virtual message window displaying all the font information.

With NVDA+F, NVDA gives information about font but there is no way one can read through the font information. This information is helpful for those who have a lot of information related to font important for them during proofreading and formatting a document.
Suggestion: this can be introduced in the form of a dialog that can be closed by just ESC.

Comment 2 by briang1 on 2015-02-13 08:54
Would this not be best done as an add on, at least in the first instance and only put into the core program if it is deemed of use by enough users.
Do people really need all of this in all editor programs, presumably even email

Comment 3 by dineshkaushal on 2015-02-20 10:46
First implementation is ready for trying.

In the current implementation, there is a problem that i am unable to fix. Escape does not work unless you move away from the dialog and come back with alt plus tab.

Checkout branch in_t4908
https://manish_agrawal@bitbucket.org/manish_agrawal/nvda.git

Comment 4 by nvdakor (in reply to comment 3) on 2015-02-20 11:10
Replying to dineshkaushal:

First implementation is ready for trying.

In the current implementation, there is a problem that i am unable to fix. Escape does not work unless you move away from the dialog and come back with alt plus tab.

Checkout branch in_t4908

https://manish_agrawal@bitbucket.org/manish_agrawal/nvda.git

Hi,
Wouldn't it be possible to use a read-only text box (perhaps available as part of WXPython) instead of generating an HTML page on the fly? That way it can be closed just by pressing the ESC key.
Thanks.

Comment 5 by leonarddr on 2015-02-20 14:48
It would probably be good to take note of how the virtual revision add-on creates its window. I agree with comment:2 though.

Comment 6 by dineshkaushal on 2015-04-09 13:54
Thanks Mick,
Your solution is working.

I have updated the fix.

Checkout branch in_t4908
​https://manish_agrawal@bitbucket.org/manish_agrawal/nvda.git

Comment 7 by dineshkaushal on 2015-08-27 13:16
HTML dialog is now working fine. Please check branch in_t4908.

now using os.path.realpath to find the full path of the htmlMessage.html file which is a template.

Comment 8 by dineshkaushal on 2015-09-30 13:07
Implemented all the suggestions provided by Jamie on the email.

==== General ====
Please rename HTMLMessage to htmlMessage wherever it occurs (lower case html at the start). Capitalisation throughout NVDA is somewhat inconsistent, but we generally try to do camel case beginning with lower case for new stuff. The only exception is type/class names which start with an upper case letter.

I have renamed the file to message.html

There is commented out code/junk in several parts of this work, as well as some unnecessary blank lines at the start/end of files. This should be removed. Please make sure you're reviewing your own diffs before submission; this would have been caught in your own review.

Removed

The documentation needs to be updated in both the docstring for the script and the User Guide to specify that pressing report formatting twice displays the information in browse mode.

Updated the docstring. Once it is accepted, will update the user guide as well.

==== HTMlMessage.html ====
The title tag should probably empty. I realise you replace this later, but right now, it's misleading because it makes it seem as if this is an untranslatable message.

Removed

The focus_away function just contains commented out code and should be removed.

Removed

Please use camel case for the function names; e.g. escapeKeyPress.

Corrected

Indentation of the js code is inconsistent and/or non-existent in various places.

Fixed

To be consistent with the rest of the NVDA code, braces should be on the same line as the statement the block is associated with.

Done

In window_onload, you split the args at semi-colons. What if the message contains a semi-colon? The split should only be done for the first semi- colon.

Fixed by adding second arguement as 2 to ensure semicolon is only used once.

message_id should probably be a div (block) instead of a span (inline).

Changed

==== globalCommands.py ====
You've added a UTF-8 BOM at the start of globalCommands.py. This will break the build. Please remove it.

Fixed

I don't think it's useful to display "No formatting information" in a dialog. I realise you're trying to be consistent here--pressing twice should always pop up a dialog--but I think this is going to be more annoying than helpful.

Now pressing nvda + f twice is showing message only when format information is available.

For HTML, I wonder if we should display each bit of the formatting info on a separate line to make it easier to consume; i.e. join with line feed instead of space for HTML. Thoughts?

I tried, but it is not working as it seems that there is only 1 element for format information. I added

etc during join operation.

==== ui.py ====
The general convention in Python is to put standard library imports first, followed by third party package imports, followed by imports for your own project. Again, this is a rule some of our existing code unfortunately doesn't follow.

Fixed

The docstring for HTMLMessage isn't useful to developers wanting to know what the function actually does. It should explain that it displays the message in an HTML document so that the user can read it with browse mode.

Corrected

The function is called HTMLMessage, but it takes a text string. This is incongruous. Should we perhaps rename this to "browseableMessage"?

Changed

There should perhaps be the option of providing HTML input; e.g. isHtml=False keyword argument.

Changed innerText to innerHTML, now html text should work

The title "NVDA Message" needs to be made translatable. Also, although this should be the default, I think it would be good if this could be customised; e.g. title=_("NVDA Message") keyword argument.

Added additional parameter to provide dialog title. I am using font as title for nvda + f, but may be it can be "format".

Please check in_t4908_fresh on https://manish_agrawal@bitbucket.org/manish_agrawal/nvda.git

I have created a new branch from master as earlier branch is 7 month old.

Comment 9 by dineshkaushal on 2015-10-07 10:08
Updated userGuide.t2t for report formatting command

Comment 10 by jteh on 2015-10-26 05:08
Thanks. Review:

GlobalCommands.script_reportFormatting

  • To get each bit of formatting info on a separate line, I suggest adding a separator keyword argument to speech.getFormatFieldSpeech which defaults to CHUNK_SEPARATOR; i.e. separator=CHUNK_SEPARATOR. This way, you can pass separator="\n" to do new lines. If you use the pre tag as I suggest below, new lines should work nicely.

message.html

  • nit: There's still a "saved from url" comment which should be removed.

ui.browseableMessage

  • I think it's important to have a way to explicitly specify plain text or HTML; e.g. isHtml=False argument. The way it is currently implemented, it's possible for an author to inject unwanted HTML by manipulating, for example, the style name. The alternative is to have all callers escape the text in all cases, but i think this is not ideal, especially since plain text will be the most common use case. Perhaps isHtml=False would use a pre tag and the innerText property, whereas isHtml=True would do what you do now.
  • nit: Get rid of the logging calls please.

User Guide

  • A user may not know what "HTML document" means, so just mention it gets displayed in browse mode.

Comment 11 by jteh on 2015-10-26 11:14
One more nit: I think the title for the reportFormatting dialog should be "Formatting", rather than "Font".

jcsteh removed the Untriaged label Nov 13, 2015

@jcsteh jcsteh added a commit that referenced this issue Dec 22, 2015

@dineshkaushal @jcsteh dineshkaushal + jcsteh Pressing the Report text formatting command twice presents the inform…
…ation in browse mode so it can be reviewed.

You can now present a text or HTML message to the user in browse mode using ui.browseableMessage.

Fixes #4908.
f11f942

Incubated in cd5ba70.

jcsteh was assigned by nvaccessAuto Dec 22, 2015

Contributor

jcsteh commented Dec 22, 2015

Pull request: #5593.

@jcsteh jcsteh added a commit that referenced this issue Dec 22, 2015

@jcsteh jcsteh ui.browseableMessage: Fix UnicodeEncodeError with Unicode strings.
Also, anything presented to the user should be unicode anyway, so change the docstring to require this.
Re #4908. Fixes #5628.
d2bed22

Incubated in 92a1bcd.

Contributor

leonardder commented Dec 23, 2015

It slightly puzzles me why this functionality is implemented this way with a HTMl template. Honestly, I prefer an implementation as proposed by Joseph in comment 4

Contributor

leonardder commented Dec 23, 2015

Also found a little bug in here:

STR:

  1. Press nvda+f twice to get the formatting info dialog
  2. Press tab
  3. Press alt twice to open and close the menu bar
    Result: An error is generated
    ERROR - unhandled exception (21:15:44): Traceback (most recent call last): File "wx\_misc.pyc", line 1367, in Notify File "wx\_core.pyc", line 16869, in Notify File "core.pyc", line 474, in _callLaterExec File "IAccessibleHandler.pyc", line 820, in _fakeFocus File "NVDAObjects\__init__.pyc", line 259, in objectWithFocus File "NVDAObjects\__init__.pyc", line 189, in findBestAPIClass File "NVDAObjects\__init__.pyc", line 189, in findBestAPIClass File "NVDAObjects\__init__.pyc", line 188, in findBestAPIClass File "NVDAObjects\IAccessible\MSHTML.pyc", line 444, in kwargsFromSuper AttributeError: 'NoneType' object has no attribute 'nodeName'
Contributor

jcsteh commented Jan 3, 2016

It slightly puzzles me why this functionality is implemented this way with a HTMl template. Honestly, I prefer an implementation as proposed by Joseph in comment 4

Using HTML allows for content other than just text; e.g. links, headings, etc. I don't see a disadvantage with this approach using plain text; you can still just press escape to close it.

I found the cause of the problem. I had set tabindex=-1 for body in message.html, so the focus was not getting there. The fix is to set tabindex=0.

I tried to checkout branch t4908, but could not find it. Making changes in in_t4908 is difficult as you made some changes after taking code from there.

Please suggest what should be done?

Contributor

jcsteh commented Feb 4, 2016

I tested with the test build given with the fix. the issue is with tab and alt key is not present now and is fixed.

@jcsteh jcsteh added a commit that referenced this issue Feb 16, 2016

@dineshkaushal @jcsteh dineshkaushal + jcsteh fixed problem in which pressing tab in html message stopped escape an…
…d there was error on pressing alt

Closes #5734. Re #4908.
86b5d8f

Incubated in 0feb4fe.

jcsteh closed this in 7f87073 Mar 8, 2016

nvaccessAuto removed the incubating label Mar 8, 2016

Collaborator

derekriemer commented Mar 9, 2016

Just a thought. Should we hash and store the hash of the message.html file in the python code? This way it is resistant to tampering. Otherwise, could it be somehow packaged somewhere where someone couldn't mess with the HTML document as easily? My concern (not that big of a deal really) is that someone could replace the message.html file on the fly in someones NVDA install, and then they could cause the message to present interesting things. For a proof of concept, go to NVDA's installation directory, (bad if it is on an external usb drive because this isn't admin privlages required), and add this code.

<script> function windowOnLoad(e){document.body.innerHTML = "proof of concept self distructing evil file.";}
function escapeKeyPress(e){document.location = "http://nvaccess.org";}
</script>

It is not a huge vonerability though.

Contributor

jcsteh commented Mar 9, 2016

Collaborator

derekriemer commented Mar 11, 2016

Yeah. That's a good point.

Collaborator

derekriemer commented Mar 11, 2016

Also, the virtual message only uses internet explorer when a link is pressed. Is this known behavior?

Contributor

leonardder commented Mar 14, 2016

Found another little bug. When rpessing nvda+f more than twice, multiple formatting windows are opened. Hold instert and press f four times, for example.

Collaborator

derekriemer commented Mar 15, 2016

In that case, it turns out that NVDA is opening a font window to display
font information (which just happens to be available) for the font
information. This works out because this is a mshtml control (I believe)
and NVDA supports retrieval of font information in those.

On 3/14/2016 9:30 AM, Leonard de Ruijter wrote:

Found another little bug. When rpessing nvda+f more than twice,
multiple formatting windows are opened. Hold instert and press f four
times, for example.


Reply to this email directly or view it on GitHub
#4908 (comment).


Derek Riemer
  • Department of computer science, third year undergraduate student.
  • Proud user of the NVDA screen reader.
  • Open source enthusiast.
  • Member of Bridge Cu
  • Avid skiier.

Websites:
Honors portfolio http://derekriemer.com
Awesome little hand built weather app!
http://django.derekriemer.com/weather/

email me at derek.riemer@colorado.edu mailto:derek.riemer@colorado.edu
Phone: (303) 906-2194

Contributor

jcsteh commented Mar 15, 2016

Also, the virtual message only uses internet explorer when a link is pressed. Is this known behavior?

Ug. That's kind of annoying. I suppose it makes sense; this function actually uses IE behind the scenes to render the HTML. The question is whether there's any way we can force it to use the shell to open external URLs. @dineshkaushal, can you look into this?

Found another little bug. When rpessing nvda+f more than twice, multiple formatting windows are opened. Hold instert and press f four times, for example.

The code displays the window if there was more than 1 press. We should instead explicitly make this 2 presses.

Collaborator

derekriemer commented Mar 16, 2016

@jcsteh
I believe this also has to do with the fact that font info can appear
for font-info window. To reproduce:

  1. Press nvda+f twice somewhere.
  2. In the window, press NVDA+f twice. It pops up font info for the
    font-info page.
  3. Pressing NVDA+f twice again produces more font-info for the font-info
    window that displays font info for the window that you originally
    requested font-info for.
    (that was a complex sentence, please excuse my non-brevity).

On 3/15/2016 5:00 PM, James Teh wrote:

Also, the virtual message only uses internet explorer when a link
is pressed. Is this known behavior?

Ug. That's kind of annoying. I suppose it makes sense; this function
actually uses IE behind the scenes to render the HTML. The question is
whether there's any way we can force it to use the shell to open
external URLs. @dineshkaushal https://github.com/Dineshkaushal, can
you look into this?

Found another little bug. When rpessing nvda+f more than twice,
multiple formatting windows are opened. Hold instert and press f
four times, for example.

The code displays the window if there was more than 1 press. We should
instead explicitly make this 2 presses.


You are receiving this because you commented.
Reply to this email directly or view it on GitHub
#4908 (comment)


Derek Riemer
  • Department of computer science, third year undergraduate student.
  • Proud user of the NVDA screen reader.
  • Open source enthusiast.
  • Member of Bridge Cu
  • Avid skiier.

Websites:
Honors portfolio http://derekriemer.com
Awesome little hand built weather app!
http://django.derekriemer.com/weather/

email me at derek.riemer@colorado.edu mailto:derek.riemer@colorado.edu
Phone: (303) 906-2194

Contributor

jcsteh commented Mar 16, 2016

Collaborator

derekriemer commented Mar 16, 2016

I was just mentioning the fact that it exists because I'm pretty sure that's what you were referencing when you were quoting it. It had looked like you had misquoted it but maybe I misunderstood what you wrote. Sorry
I would also argue that it is not a book as well.

Sent from my heavily encrypted iPhone.
Please disregard errors as this is a smaller device.

On Mar 15, 2016, at 22:12, James Teh notifications@github.com wrote:

Sure, but I don't consider this to be a bug. If you want formatting info
for your formatting info, that's your business. It's a valid (if stupid)
use case. Unless you can give me some reason it isn't valid...

On the other hand, opening another dialog when you press it three times
is not valid, since it's only documented to do it when you press it
twice. It's also more likely to be accidentally triggered due to a
finger slip or the like.

You are receiving this because you commented.
Reply to this email directly or view it on GitHub

Contributor

leonardder commented Mar 16, 2016

Contributor

jcsteh commented Mar 16, 2016

Contributor

leonardder commented Mar 16, 2016

Contributor

jcsteh commented Mar 18, 2016

@jcsteh jcsteh added a commit that referenced this issue Mar 18, 2016

@jcsteh jcsteh Report formatting command: Only open a formatting info dialog for two…
… presses; i.e. don't allow the third press, etc. to open another dialog.

Re #4908.
856a06a
Contributor

tspivey commented Mar 26, 2016

Why does this say formatting 2 or 3 times?

IO - inputCore.InputManager.executeGesture (19:48:49):
Input: kb(laptop):NVDA+f
IO - speech.speak (19:48:49):
Speaking [u'Dark Gray on Black']
IO - inputCore.InputManager.executeGesture (19:48:49):
Input: kb(laptop):NVDA+f
IO - speech.speak (19:48:49):
Speaking [u'Formatting']
IO - speech.speak (19:48:50):
Speaking [u'Formatting']
IO - speech.speak (19:48:50):
Speaking [IndexCommand(1), u'Dark Gray on Black']
Contributor

nishimotz commented Apr 29, 2016

The 'center' option for HTML dialog is specified, however, the actual dialog position is not center on screen.
"center:desktop" seems work.

diff --git a/source/ui.py b/source/ui.py
index c91d0d0..862c782 100644
--- a/source/ui.py
+++ b/source/ui.py
@@ -23,7 +23,7 @@ import braille
 URL_MK_UNIFORM = 1

 # Dialog box properties
-DIALOG_OPTIONS = "dialogWidth:350px;dialogHeight:140px;resizable:yes;center:yes;help:no"
+DIALOG_OPTIONS = "dialogWidth:350px;dialogHeight:140px;resizable:yes;center:desktop;help:no"

 #dwDialogFlags for ShowHTMLDialogEx from mshtmhst.h
 HTMLDLG_NOUI = 0x0010
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment