Support the HTML5 required attribute in MSHTML documents

[LeonarddeR]

Link to issue number:

As far as I know, this pr has no dedicated issue for itself. This pr is based on findings in #7320 (comment)

Summary of the issue:

NVDA does not report the required state of a html element when the HTML5 required attribute is used for this.

Description of how this pull request fixes the issue:

Added support for required in de NVDAHelper MSHTML vbuf backend and associated python code.

Testing performed:

For required

<input id="name" required type="text" />

For not required

<input id="name" type="text" />

Note that required="false" will still result in required state being added, but this is no bug according to the spec. I've seen the same behaviour in Firefox.

Change log entry:

I consider the absence of this feature to be a bug.

• Bug fixes
  • In MSHTML documents, the HTML5 required attribute is now supported to indicate the required state of a form field (Support the HTML5 required attribute in MSHTML documents #7321)

[michaelDCurran]

This looks good so far, but can you please also support this in focus mode. See aria-required in NVDAObjects\IAccessible\MSHTML.py

[LeonarddeR]

Hmm, good point. However, it seems this is going to be more difficult than I thought

<input type="text" required="required" name="cheese" />

>>> nav.HTMLNode.getAttribute('type')
u'text'
>>> nav.HTMLNode.getAttribute('name')
u'cheese'
>>> type(nav.HTMLNode.getAttribute('required'))
<type 'NoneType'>

It seems the node doesn't expose the required attribute correctly.

[LeonarddeR]

A, it turns out that I can use HTMLNode.hasAttribute('required')

[LeonarddeR]

@michaelDCurran: this is now working.

I added a contains method to the HTMLAttribCache to support stuff like 'required' in self.HTMLAttributes. I decided to give it a separate cache, as I didn't want to change the behaviour of getitem. This nieuw contains_ method fixes a bug where 'attribute' in HTMLAttributes resulted in a freeze of NVDA.

[michaelDCurran]

This can be a set rather than a dictionary

[LeonarddeR]

Mm, not sure whether we're on the same track here. Changing this to a set means that we lose track of the attributes for which hasAttribute has returned False in the past, so in other words, False evaluations won't get cached. We either need a dictionary for this, or have two sets for hasAttribute True and hasAttribute False. Could it be that you overlooked this, or is it your intention not to cache the Falses?

[michaelDCurran]

Then change this to return item in self.containsCache

[LeonarddeR]

If we do this, the block below that actually runs hasAttribute never gets executed.

[michaelDCurran]

I would perhaps only check item in self.containsCache, then item in self.cache, and if none of them then node.hasattribute(item) and add the answer to containsCache. hasattribute should work for any value that getattribute would work for.

[michaelDCurran]

Make sure to test on several versions of Internet Explorer. At very least what ever Internet Explorer comes with Windows 7.

[LeonarddeR]

This is a good point. I hope I have a proper vm around for this.

[LeonarddeR]

@michaelDCurran: Would you be able to explain why you belief containsCache should be a set rather than a dictionary? See #7321 (comment)

If you agree with containsCache being a dictionary anyway, this is ready for another review.

[michaelDCurran]

This looks good to me. Ignore the set/dictionary comment.

[michaelDCurran]

@LeonarddeR If you are finnished with this I'll incubate it?

[LeonarddeR]

@michaelDCurran commented on 19 jul. 2017 03:37 CEST:

@LeonarddeR If you are finnished with this I'll incubate it?

Yes, go ahead. Thanks a lot!