Significantly speed up heading quick navigation in Microsoft Edge by walking heading Text elements rather than looking at all paragraphs #7343
Conversation
… walking heading Text elements rather than looking at all paragraphs.
|
What is this expected to do for div elements that have an role of heading but don't have the aria-level set? I know this sounds like the edge case of edge cases, but I've seen these in the wild.
|
|
That's cutting edge code. (Derek hides).
…On Thu, Jun 29, 2017 at 11:41 PM, Leonard de Ruijter < ***@***.***> wrote:
What is this expected to do for div elements that have an role of heading
but don't have the aria-level set? I know this sounds like the edge case of
edge cases, but I've seen these in the wild.
—
You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub
<#7343 (comment)>, or mute
the thread
<https://github.com/notifications/unsubscribe-auth/AFGivakZ8Gq0u2QO1chfOkKqSx6ffRc7ks5sJIp0gaJpZM4OKKIS>
.
--
Derek Riemer: Improving the world one byte at a time!
- University of Colorado Boulder Department of computer science, 4th
year undergraduate student.
- Accessibility enthusiast.
- Proud user of the NVDA screen reader.
- Open source enthusiast.
- Skier.
Personal website <http://derekriemer.com>
|
| def isChild(self,parent): | ||
| return self.level>parent.level | ||
|
|
||
| def EdgeHeadingQuicknavIterator(itemType,document,position,direction="next"): |
There was a problem hiding this comment.
Could you add a doc string here? This isn't an inherited interface is it?
There was a problem hiding this comment.
The docstring can be found on browseMode.QuickNavItem.isChild
There was a problem hiding this comment.
Sorry I meant on EdgeHeadingQuicknavIterator. As an aside, how would you feel about adding doc strings on derived class functions to say something like @see parentclass.methodname. This would make it clearer that this is an override, and where to find the documentation for it.
| levelString=itemType[7:] | ||
| for item in UIAControlQuicknavIterator(itemType,document,position,condition,direction=direction,itemClass=EdgeHeadingQuickNavItem): | ||
| # Verify this is the correct heading level via text attributes | ||
| if item.level and (not levelString or levelString==str(item.level)): |
There was a problem hiding this comment.
Is this to catch mistakes in what Edge returns? Do we have any examples of where this is required?
There was a problem hiding this comment.
The UIA search condition searches for an element with a controlType of text, and some kind of level. Edge itself does not provide anything more specific to search for. Thus, once we do have an element and have placed it in a headingQuickNavItem we then need to verify it is actually a heading by checking its level property. HeadingQuicknavItem.level is only valid if the element is in deed a valid heading (I.e. it also checks some text attributes).
|
what about this invalid, but still seen in the wild code? Will this PR break heading nav to these? I see this at least once a month, and it is incorrect, but it is nice to be able to move by heading with them still. |
|
@derekriemer: Edge currently assigns these a level of 1, so they will be picked up. |
Summary of the issue:
Currently navigating by heading in Microsoft Edge is extremely slow on large pages.
For example, when on the World War I wikipedia article in Edge, pressing 1 (to jump to the next heading level 1) can take up to 20 seconds to report there are no more heading ones.
Similarly, jumping heading by heading through the page with h can sometimes take up to 5 seconds locating the next heading.
The current way this is implemented is that NVDA moves through the document by paragraph and checks the text attributes to see if this paragraph is a heading.
Description of how this pull request fixes the issue:
As of the Windows Anniversary update, Edge started exposing an real UIA element for each heading. It can be identified with a controlType of text, and a level between 1 and 6.
Therefore, this pull request walks the UIA tree requesting only these elements. When one is found however, it still double checks the text attributes as sometimes the level on the element can be wrong.
Even so, this significantly speeds up heading navigation. Searching for a heading 1 in the World War I article now takes about 1 to 2 seconds rather than 20.
Change log entry:
Bug fixes: