displaying font information in a virtual message will be helpful.

nvaccessAuto commented 9 years ago

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.

nvaccessAuto commented 9 years ago

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

nvaccessAuto commented 9 years ago

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

nvaccessAuto commented 9 years ago

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.

nvaccessAuto commented 9 years ago

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.

nvaccessAuto commented 9 years ago

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

nvaccessAuto commented 9 years ago

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.

nvaccessAuto commented 9 years ago

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.

nvaccessAuto commented 9 years ago

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

nvaccessAuto commented 9 years ago

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.

nvaccessAuto commented 9 years ago

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".

nvaccessAuto commented 8 years ago

Incubated in cd5ba7031fe26f1e26d087e6202b9d7b6d9a5543.

jcsteh commented 8 years ago

Pull request: #5593.

nvaccessAuto commented 8 years ago

Incubated in 92a1bcdbe10028602d1eb59863f389c0ed50f54d.

LeonarddeR commented 8 years ago

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

LeonarddeR commented 8 years ago

Also found a little bug in here:

STR:

Press nvda+f twice to get the formatting info dialog
Press tab
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'

jcsteh commented 8 years ago

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.

dineshkaushal commented 8 years ago

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?

jcsteh commented 8 years ago

The branch is called i4908 in the nvaccess/nvda repository. We use "i" for issue now instead of "t" for ticket.

sumandogra commented 8 years ago

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.

nvaccessAuto commented 8 years ago

Incubated in 0feb4fe5ec727bdf5b9d1fc4795eeb08348d323f.

derekriemer commented 8 years ago

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.

jcsteh commented 8 years ago

Someone could certainly do this, yes. However, they could also just replace a dll, executable or some other file. Unless I'm missing something, there's nothing particularly special about this file. If we're worrying about someone replacing this, one must equally worry about any file being replaced anywhere for any app.

derekriemer commented 8 years ago

Yeah. That's a good point.

derekriemer commented 8 years ago

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

LeonarddeR commented 8 years ago

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.

derekriemer commented 8 years ago

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 https://github.com/nvaccess/nvda/issues/4908#issuecomment-196369022.

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

jcsteh commented 8 years ago

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.

derekriemer commented 8 years ago

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

Press nvda+f twice somewhere.
In the window, press NVDA+f twice. It pops up font info for the font-info page.
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 https://github.com/nvaccess/nvda/issues/4908#issuecomment-197060974

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

jcsteh commented 8 years ago

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.

derekriemer commented 8 years ago

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

LeonarddeR commented 8 years ago

Shouldn't we actually just block people from opening multiple formatting information windows in a simlar way we do with configuration dialogs?

jcsteh commented 8 years ago

Well, stuff other than formatting dialogs could use browseable messages, so that question has to be broadened to any browseable message. Honestly, I don't know if there's scope for allowing multiple messages to be open at once. It doesn't seem like we should block it just for the sake of it.

Multiple settings dialogs are only prevented because settings can depend on each other. For example, if you open both Voice Settings and Synthesizer, change your synthesizer and then try to use the Voice Settings dialog, things will be broken because Voice Settings doesn't know about the synth change (#603).

LeonarddeR commented 8 years ago

I see your point. However, multiple formatting windows create confusion about what window is for what formatting. Couldn't multiple formatting windows be disabled regardless of other browsable messages?

jcsteh commented 8 years ago

Because the formatting dialog is (and must be) modeless, it isn't trivial to prevent more than one dialog. In fact, I'm not even sure whether there's a way to get a callback from that dialog when it closes.

tspivey commented 8 years ago

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']

nishimotz commented 8 years ago

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

nvaccess / nvda