Skip to content

Commit

Permalink
Bug 1611060 - Dispatch a11y scroll events for all DOM scrolls. r=Jamie
Browse files Browse the repository at this point in the history 
Currently, only documents dispatch scroll events when in reality any
element can have scrollable overflows.

Differential Revision: https://phabricator.services.mozilla.com/D62665

--HG--
extra : moz-landing-system : lando
  • Loading branch information
@eeejay
eeejay committed Feb 17, 2020
1 parent 171ffe8 commit a98323b
Show file tree
Hide file tree
Showing 6 changed files with 93 additions and 82 deletions.
4 changes: 0 additions & 4 deletions accessible/base/NotificationController.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -638,10 +638,6 @@ void NotificationController::WillRefresh(mozilla::TimeStamp aTime) {
"isn't created!");
}

// Initialize scroll support if needed.
if (!(mDocument->mDocFlags & DocAccessible::eScrollInitialized))
mDocument->AddScrollListener();

// Process rendered text change notifications.
for (auto iter = mTextHash.Iter(); !iter.Done(); iter.Next()) {
nsCOMPtrHashKey<nsIContent>* entry = iter.Get();
Expand Down
21 changes: 0 additions & 21 deletions accessible/generic/DocAccessible-inl.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -77,27 +77,6 @@ inline void DocAccessible::UpdateText(nsIContent* aTextNode) {
mNotificationController->ScheduleTextUpdate(aTextNode);
}

inline void DocAccessible::AddScrollListener() {
// Delay scroll initializing until the document has a root frame.
if (!mPresShell->GetRootFrame()) return;

mDocFlags |= eScrollInitialized;
nsIScrollableFrame* sf = mPresShell->GetRootScrollFrameAsScrollable();
if (sf) {
sf->AddScrollPositionListener(this);

#ifdef A11Y_LOG
if (logging::IsEnabled(logging::eDocCreate))
logging::Text("add scroll listener");
#endif
}
}

inline void DocAccessible::RemoveScrollListener() {
nsIScrollableFrame* sf = mPresShell->GetRootScrollFrameAsScrollable();
if (sf) sf->RemoveScrollPositionListener(this);
}

inline void DocAccessible::NotifyOfLoad(uint32_t aLoadEventType) {
mLoadState |= eDOMLoaded;
mLoadEventType = aLoadEventType;
Expand Down
71 changes: 42 additions & 29 deletions accessible/generic/DocAccessible.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -81,7 +81,6 @@ DocAccessible::DocAccessible(dom::Document* aDocument,
mAccessibleCache(kDefaultCacheLength),
mNodeToAccessibleMap(kDefaultCacheLength),
mDocumentNode(aDocument),
mScrollPositionChangedTicks(0),
mLoadState(eTreeConstructionPending),
mDocFlags(0),
mLoadEventType(0),
Expand Down Expand Up @@ -510,9 +509,6 @@ nsresult DocAccessible::AddEventListeners() {
// DocAccessible protected member
nsresult DocAccessible::RemoveEventListeners() {
// Remove listeners associated with content documents
// Remove scroll position listener
RemoveScrollListener();

NS_ASSERTION(mDocumentNode, "No document during removal of listeners.");

if (mDocumentNode) {
Expand Down Expand Up @@ -545,7 +541,14 @@ void DocAccessible::ScrollTimerCallback(nsITimer* aTimer, void* aClosure) {
DocAccessible* docAcc = reinterpret_cast<DocAccessible*>(aClosure);

if (docAcc) {
docAcc->DispatchScrollingEvent(nsIAccessibleEvent::EVENT_SCROLLING_END);
// Dispatch a scroll-end for all entries in table. They have not
// been scrolled in at least `kScrollEventInterval`.
for (auto iter = docAcc->mLastScrollingDispatch.Iter(); !iter.Done();
iter.Next()) {
docAcc->DispatchScrollingEvent(iter.Key(),
nsIAccessibleEvent::EVENT_SCROLLING_END);
iter.Remove();
}

if (docAcc->mScrollWatchTimer) {
docAcc->mScrollWatchTimer = nullptr;
Expand All @@ -554,17 +557,16 @@ void DocAccessible::ScrollTimerCallback(nsITimer* aTimer, void* aClosure) {
}
}

////////////////////////////////////////////////////////////////////////////////
// nsIScrollPositionListener

void DocAccessible::ScrollPositionDidChange(nscoord aX, nscoord aY) {
void DocAccessible::HandleScroll(nsINode* aTarget) {
const uint32_t kScrollEventInterval = 100;
TimeStamp timestamp = TimeStamp::Now();
if (mLastScrollingDispatch.IsNull() ||
(timestamp - mLastScrollingDispatch).ToMilliseconds() >=
kScrollEventInterval) {
DispatchScrollingEvent(nsIAccessibleEvent::EVENT_SCROLLING);
mLastScrollingDispatch = timestamp;
TimeStamp now = TimeStamp::Now();
TimeStamp lastDispatch;
// If we haven't dispatched a scrolling event for a target in at least
// kScrollEventInterval milliseconds, dispatch one now.
if (!mLastScrollingDispatch.Get(aTarget, &lastDispatch) ||
(now - lastDispatch).ToMilliseconds() >= kScrollEventInterval) {
DispatchScrollingEvent(aTarget, nsIAccessibleEvent::EVENT_SCROLLING);
mLastScrollingDispatch.Put(aTarget, now);
}

// If timer callback is still pending, push it 100ms into the future.
Expand Down Expand Up @@ -2522,27 +2524,38 @@ void DocAccessible::SetIPCDoc(DocAccessibleChild* aIPCDoc) {
mIPCDoc = aIPCDoc;
}

void DocAccessible::DispatchScrollingEvent(uint32_t aEventType) {
nsIScrollableFrame* sf = mPresShell->GetRootScrollFrameAsScrollable();
if (!sf) {
void DocAccessible::DispatchScrollingEvent(nsINode* aTarget,
uint32_t aEventType) {
Accessible* acc = GetAccessible(aTarget);
if (!acc) {
return;
}

int32_t appUnitsPerDevPixel =
mPresShell->GetPresContext()->AppUnitsPerDevPixel();
LayoutDevicePoint scrollPoint =
LayoutDevicePoint::FromAppUnits(sf->GetScrollPosition(),
appUnitsPerDevPixel) *
mPresShell->GetResolution();
LayoutDevicePoint scrollPoint;
LayoutDeviceRect scrollRange;
nsIScrollableFrame* sf = acc == this
? mPresShell->GetRootScrollFrameAsScrollable()
: acc->GetFrame()->GetScrollTargetFrame();

// If there is no scrollable frame, it's likely a scroll in a popup, like
// <select>. Just send an event with no scroll info. The scroll info
// is currently only used on Android, and popups are rendered natively
// there.
if (sf) {
int32_t appUnitsPerDevPixel =
mPresShell->GetPresContext()->AppUnitsPerDevPixel();
scrollPoint = LayoutDevicePoint::FromAppUnits(sf->GetScrollPosition(),
appUnitsPerDevPixel) *
mPresShell->GetResolution();

LayoutDeviceRect scrollRange =
LayoutDeviceRect::FromAppUnits(sf->GetScrollRange(), appUnitsPerDevPixel);
scrollRange.ScaleRoundOut(mPresShell->GetResolution());
scrollRange = LayoutDeviceRect::FromAppUnits(sf->GetScrollRange(),
appUnitsPerDevPixel);
scrollRange.ScaleRoundOut(mPresShell->GetResolution());
}

RefPtr<AccEvent> event =
new AccScrollingEvent(aEventType, this, scrollPoint.x, scrollPoint.y,
new AccScrollingEvent(aEventType, acc, scrollPoint.x, scrollPoint.y,
scrollRange.width, scrollRange.height);

nsEventShell::FireEvent(event);
}

Expand Down
29 changes: 10 additions & 19 deletions accessible/generic/DocAccessible.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,6 @@
#include "mozilla/dom/Document.h"
#include "nsIDocumentObserver.h"
#include "nsIObserver.h"
#include "nsIScrollPositionListener.h"
#include "nsITimer.h"

class nsAccessiblePivot;
Expand Down Expand Up @@ -45,7 +44,6 @@ class TNotification;
class DocAccessible : public HyperTextAccessibleWrap,
public nsIDocumentObserver,
public nsIObserver,
public nsIScrollPositionListener,
public nsSupportsWeakReference,
public nsIAccessiblePivotObserver {
NS_DECL_ISUPPORTS_INHERITED
Expand All @@ -60,10 +58,6 @@ class DocAccessible : public HyperTextAccessibleWrap,
public:
DocAccessible(Document* aDocument, PresShell* aPresShell);

// nsIScrollPositionListener
virtual void ScrollPositionWillChange(nscoord aX, nscoord aY) override {}
virtual void ScrollPositionDidChange(nscoord aX, nscoord aY) override;

// nsIDocumentObserver
NS_DECL_NSIDOCUMENTOBSERVER

Expand Down Expand Up @@ -384,6 +378,13 @@ class DocAccessible : public HyperTextAccessibleWrap,
*/
DocAccessibleChild* IPCDoc() const { return mIPCDoc; }

/**
* Notify the document that a DOM node has been scrolled. document will
* dispatch throttled accessibility events for scrolling, and a scroll-end
* event.
*/
void HandleScroll(nsINode* aTarget);

protected:
virtual ~DocAccessible();

Expand Down Expand Up @@ -418,12 +419,6 @@ class DocAccessible : public HyperTextAccessibleWrap,
*/
void ProcessLoad();

/**
* Add/remove scroll listeners, @see nsIScrollPositionListener interface.
*/
void AddScrollListener();
void RemoveScrollListener();

/**
* Append the given document accessible to this document's child document
* accessibles.
Expand Down Expand Up @@ -577,7 +572,7 @@ class DocAccessible : public HyperTextAccessibleWrap,
*/
static void ScrollTimerCallback(nsITimer* aTimer, void* aClosure);

void DispatchScrollingEvent(uint32_t aEventType);
void DispatchScrollingEvent(nsINode* aTarget, uint32_t aEventType);

/**
* Check if an id attribute change affects aria-activedescendant and handle
Expand Down Expand Up @@ -607,11 +602,8 @@ class DocAccessible : public HyperTextAccessibleWrap,
* State and property flags, kept by mDocFlags.
*/
enum {
// Whether scroll listeners were added.
eScrollInitialized = 1 << 0,

// Whether the document is a tab document.
eTabDocument = 1 << 1
eTabDocument = 1 << 0
};

/**
Expand All @@ -623,8 +615,7 @@ class DocAccessible : public HyperTextAccessibleWrap,

Document* mDocumentNode;
nsCOMPtr<nsITimer> mScrollWatchTimer;
uint16_t mScrollPositionChangedTicks; // Used for tracking scroll events
TimeStamp mLastScrollingDispatch;
nsDataHashtable<nsPtrHashKey<nsINode>, TimeStamp> mLastScrollingDispatch;

/**
* Bit mask of document load states (@see LoadState).
Expand Down
26 changes: 18 additions & 8 deletions accessible/generic/RootAccessible.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -144,7 +144,7 @@ const char* const kEventTypes[] = {
// HTMLInputElement.cpp & radio.js)
"RadioStateChange", "popupshown", "popuphiding", "DOMMenuInactive",
"DOMMenuItemActive", "DOMMenuItemInactive", "DOMMenuBarActive",
"DOMMenuBarInactive"};
"DOMMenuBarInactive", "scroll"};

nsresult RootAccessible::AddEventListeners() {
// EventTarget interface allows to register event listeners to
Expand Down Expand Up @@ -222,13 +222,23 @@ RootAccessible::HandleEvent(Event* aDOMEvent) {
GetAccService()->GetDocAccessible(origTargetNode->OwnerDoc());

if (document) {
// Root accessible exists longer than any of its descendant documents so
// that we are guaranteed notification is processed before root accessible
// is destroyed.
// For shadow DOM, GetOriginalTarget on the Event returns null if we
// process the event async, so we must pass the target node as well.
document->HandleNotification<RootAccessible, Event, nsINode>(
this, &RootAccessible::ProcessDOMEvent, aDOMEvent, origTargetNode);
nsAutoString eventType;
aDOMEvent->GetType(eventType);
if (eventType.EqualsLiteral("scroll")) {
// We don't put this in the notification queue for 2 reasons:
// 1. We will flood the queue with repetitive events.
// 2. Since this doesn't necessarily touch layout, we are not
// guaranteed to have a WillRefresh tick any time soon.
document->HandleScroll(origTargetNode);
} else {
// Root accessible exists longer than any of its descendant documents so
// that we are guaranteed notification is processed before root accessible
// is destroyed.
// For shadow DOM, GetOriginalTarget on the Event returns null if we
// process the event async, so we must pass the target node as well.
document->HandleNotification<RootAccessible, Event, nsINode>(
this, &RootAccessible::ProcessDOMEvent, aDOMEvent, origTargetNode);
}
}

return NS_OK;
Expand Down
24 changes: 23 additions & 1 deletion accessible/tests/browser/events/browser_test_scrolling.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,9 @@ addAccessibleTask(
`
<div style="height: 100vh" id="one">one</div>
<div style="height: 100vh" id="two">two</div>
<div style="height: 100vh; width: 200vw" id="three">three</div>`,
<div style="height: 100vh; width: 200vw; overflow: auto;" id="three">
<div style="height: 300%;">three</div>
</div>`,
async function(browser, accDoc) {
let onScrolling = waitForEvents([
[EVENT_SCROLLING, accDoc],
Expand Down Expand Up @@ -70,5 +72,25 @@ addAccessibleTask(
scrollEvent3.scrollX > scrollEvent2.scrollX,
`${scrollEvent3.scrollX} > ${scrollEvent2.scrollX}`
);

// non-doc scrolling
onScrolling = waitForEvents([
[EVENT_SCROLLING, "three"],
[EVENT_SCROLLING_END, "three"],
]);
await SpecialPowers.spawn(browser, [], () => {
content.document.querySelector("#three").scrollTo(0, 10);
});
let [scrollEvent4, scrollEndEvent4] = await onScrolling;
scrollEvent4.QueryInterface(nsIAccessibleScrollingEvent);
ok(
scrollEvent4.maxScrollY >= scrollEvent4.scrollY,
"scrollY is within max"
);
scrollEndEvent4.QueryInterface(nsIAccessibleScrollingEvent);
ok(
scrollEndEvent4.maxScrollY >= scrollEndEvent4.scrollY,
"scrollY is within max"
);
}
);

0 comments on commit a98323b

Please sign in to comment.