Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Significantly speed up heading quick navigation in Microsoft Edge by walking heading Text elements rather than looking at all paragraphs #7343

Merged
merged 2 commits into from Jul 25, 2017
Merged

Significantly speed up heading quick navigation in Microsoft Edge by walking heading Text elements rather than looking at all paragraphs #7343

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
d2b8c02
Significantly speed up heading quick navigation in Microsoft Edge but…
michaelDCurran Jun 30, 2017
eac1e6f
Address review comments.
michaelDCurran Jul 11, 2017
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
37 changes: 36 additions & 1 deletion source/NVDAObjects/UIA/edge.py
View file
Open in desktop
Expand Up @@ -17,7 +17,7 @@
import aria
import textInfos
import UIAHandler
from UIABrowseMode import UIABrowseModeDocument, UIABrowseModeDocumentTextInfo
from UIABrowseMode import UIABrowseModeDocument, UIABrowseModeDocumentTextInfo, UIATextRangeQuickNavItem,UIAControlQuicknavIterator
from UIAUtils import *
from . import UIA, UIATextInfo

Expand Down Expand Up @@ -508,13 +508,48 @@ def event_gainFocus(self):
return
return super(EdgeHTMLRootContainer,self).event_gainFocus()

class EdgeHeadingQuickNavItem(UIATextRangeQuickNavItem):

@property
def level(self):
if not hasattr(self,'_level'):
styleVal=getUIATextAttributeValueFromRange(self.textInfo._rangeObj,UIAHandler.UIA_StyleIdAttributeId)
self._level=styleVal-(UIAHandler.StyleId_Heading1-1) if UIAHandler.StyleId_Heading1<=styleVal<=UIAHandler.StyleId_Heading6 else None
return self._level

def isChild(self,parent):
return self.level>parent.level

def EdgeHeadingQuicknavIterator(itemType,document,position,direction="next"):
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you add a doc string here? This isn't an inherited interface is it?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The docstring can be found on browseMode.QuickNavItem.isChild

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

"""
A helper for L{EdgeHTMLTreeInterceptor._iterNodesByType} that specifically yields L{EdgeHeadingQuickNavItem} objects found in the given document, starting the search from the given position, searching in the given direction.
See L{browseMode._iterNodesByType} for details on these specific arguments.
"""
# Edge exposes all headings as UIA elements with a controlType of text, and a level. Thus we can quickly search for these.
# However, sometimes when ARIA is used, the level on the element may not match the level in the text attributes.
# Therefore we need to search for all levels 1 through 6, even if a specific level is specified.
# Though this is still much faster than searching text attributes alone
levels=range(1,7)
condition=createUIAMultiPropertyCondition({UIAHandler.UIA_ControlTypePropertyId:UIAHandler.UIA_TextControlTypeId,UIAHandler.UIA_LevelPropertyId:levels})
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)):
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this to catch mistakes in what Edge returns? Do we have any examples of where this is required?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

yield item

class EdgeHTMLTreeInterceptor(cursorManager.ReviewCursorManager,UIABrowseModeDocument):

TextInfo=UIABrowseModeDocumentTextInfo

def _get_documentConstantIdentifier(self):
return self.rootNVDAObject.parent.name

def _iterNodesByType(self,nodeType,direction="next",pos=None):
if nodeType.startswith("heading"):
return EdgeHeadingQuicknavIterator(nodeType,self,pos,direction=direction)
else:
return super(EdgeHTMLTreeInterceptor,self)._iterNodesByType(nodeType,direction=direction,pos=pos)

def shouldPassThrough(self,obj,reason=None):
# Enter focus mode for selectable list items (<select> and role=listbox)
if reason==controlTypes.REASON_FOCUS and obj.role==controlTypes.ROLE_LISTITEM and controlTypes.STATE_SELECTABLE in obj.states:
Expand Down
16 changes: 8 additions & 8 deletions source/UIABrowseMode.py
View file
Open in desktop
Expand Up @@ -78,7 +78,7 @@ def UIAHeadingQuicknavIterator(itemType,document,position,direction="next"):
stop=(curPosition.move(textInfos.UNIT_PARAGRAPH,1 if direction=="next" else -1)==0)
firstLoop=False

def UIAControlQuicknavIterator(itemType,document,position,UIACondition,direction="next"):
def UIAControlQuicknavIterator(itemType,document,position,UIACondition,direction="next",itemClass=UIATextRangeQuickNavItem):
# A part from the condition given, we must always match on the root of the document so we know when to stop walking
runtimeID=VARIANT()
document.rootNVDAObject.UIAElement._IUIAutomationElement__com_GetCurrentPropertyValue(UIAHandler.UIA_RuntimeIdPropertyId,byref(runtimeID))
Expand All @@ -94,14 +94,14 @@ def UIAControlQuicknavIterator(itemType,document,position,UIACondition,direction
except COMError:
elementRange=None
if elementRange:
yield UIATextRangeQuickNavItem(itemType,document,elementRange)
yield itemClass(itemType,document,elementRange)
return
if direction=="up":
walker=UIAHandler.handler.clientObject.createTreeWalker(UIACondition)
element=position.UIAElementAtStart
element=walker.normalizeElement(element)
if element and not UIAHandler.handler.clientObject.compareElements(element,document.rootNVDAObject.UIAElement) and not UIAHandler.handler.clientObject.compareElements(element,UIAHandler.handler.rootElement):
yield UIATextRangeQuickNavItem(itemType,document,element)
yield itemClass(itemType,document,element)
return
elif direction=="previous":
# Fetching items previous to the given position.
Expand Down Expand Up @@ -159,7 +159,7 @@ def UIAControlQuicknavIterator(itemType,document,position,UIACondition,direction
elif not curElementMatchedCondition and isUIAElementInWalker(curElement,walker):
curElementMatchedCondition=True
if curElementMatchedCondition:
yield UIATextRangeQuickNavItem(itemType,document,curElement)
yield itemClass(itemType,document,curElement)
previousSibling=walker.getPreviousSiblingElement(curElement)
if previousSibling:
gonePreviousOnce=True
Expand All @@ -173,7 +173,7 @@ def UIAControlQuicknavIterator(itemType,document,position,UIACondition,direction
goneParent=True
curElementMatchedCondition=True
if gonePreviousOnce:
yield UIATextRangeQuickNavItem(itemType,document,curElement)
yield itemClass(itemType,document,curElement)
continue
curElement=None
else: # direction is next
Expand Down Expand Up @@ -220,13 +220,13 @@ def UIAControlQuicknavIterator(itemType,document,position,UIACondition,direction
# If we are already past our position, and this is a valid child
# Then we can emit an item already
if goneNextOnce and isUIAElementInWalker(curElement,walker):
yield UIATextRangeQuickNavItem(itemType,document,curElement)
yield itemClass(itemType,document,curElement)
# Start traversing from this child forwards through the document, emitting items for valid elements.
while curElement:
firstChild=walker.getFirstChildElement(curElement) if goneNextOnce else None
if firstChild:
curElement=firstChild
yield UIATextRangeQuickNavItem(itemType,document,curElement)
yield itemClass(itemType,document,curElement)
else:
nextSibling=None
while not nextSibling:
Expand All @@ -239,7 +239,7 @@ def UIAControlQuicknavIterator(itemType,document,position,UIACondition,direction
return
curElement=nextSibling
goneNextOnce=True
yield UIATextRangeQuickNavItem(itemType,document,curElement)
yield itemClass(itemType,document,curElement)

class UIABrowseModeDocumentTextInfo(browseMode.BrowseModeDocumentTextInfo,treeInterceptorHandler.RootProxyTextInfo):

Expand Down