Skip to content

Commit

Permalink
[A11y] Pass in parent when creating AXObject for popup document
Browse files Browse the repository at this point in the history
In general, it's better to supply the parent when creating AXObjects.
In cases where it isn't supplied, we have to try to repair the local
included subtree. Necessary for landing CL:4873421, which is a
refactoring of how we do repairs.

Bug: none
Change-Id: I2f70edf95074d79bdc5ddad215d4b6f73dfb9bf6
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4969292
Reviewed-by: Chris Harrelson <chrishtr@chromium.org>
Commit-Queue: Aaron Leventhal <aleventhal@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1216763}
  • Loading branch information
aleventhal authored and Chromium LUCI CQ committed Oct 30, 2023
1 parent a1f3352 commit 9f8b3b4
Show file tree
Hide file tree
Showing 18 changed files with 31 additions and 26 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,6 @@

namespace blink {

class LayoutObject;
class AXObject;

// AXObjectCacheBase is a temporary class that sits between AXObjectCache and
Expand All @@ -26,9 +25,11 @@ class CORE_EXPORT AXObjectCacheBase : public AXObjectCache {
AXObjectCacheBase& operator=(const AXObjectCacheBase&) = delete;
~AXObjectCacheBase() override = default;

virtual AXObject* GetOrCreate(LayoutObject*,
virtual AXObject* GetOrCreate(const Node*,
AXObject* parent_if_known = nullptr) = 0;

virtual AXObject* Get(const Node*) = 0;

protected:
AXObjectCacheBase() = default;
};
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -573,17 +573,17 @@ void WebPagePopupImpl::DestroyPage() {
page_.Clear();
}

AXObject* WebPagePopupImpl::RootAXObject() {
AXObject* WebPagePopupImpl::RootAXObject(Element* popup_owner) {
if (!page_)
return nullptr;
// If |page_| is non-null, the main frame must have a Document.
Document* document = MainFrame().GetDocument();
AXObjectCache* cache = document->ExistingAXObjectCache();
AXObjectCacheBase* cache =
To<AXObjectCacheBase>(document->ExistingAXObjectCache());
// There should never be a circumstance when RootAXObject() is triggered
// and the AXObjectCache doesn't already exist. It's called when trying
// to attach the accessibility tree of the pop-up to the host page.
DCHECK(cache);
return To<AXObjectCacheBase>(cache)->GetOrCreate(document->GetLayoutView());
return cache->GetOrCreate(document, cache->Get(popup_owner));
}

void WebPagePopupImpl::SetWindowRect(const gfx::Rect& rect_in_screen) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -222,7 +222,7 @@ class CORE_EXPORT WebPagePopupImpl final : public WebPagePopup,
gfx::Rect GetAnchorRectInScreen() const;

// PagePopup function
AXObject* RootAXObject() override;
AXObject* RootAXObject(Element* popover_owner) override;
void SetWindowRect(const gfx::Rect&) override;

WebPagePopupImpl(
Expand Down
3 changes: 2 additions & 1 deletion third_party/blink/renderer/core/html/forms/color_chooser.h
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@
namespace blink {

class AXObject;
class Element;
class Color;

// This interface respresents a UI to choose a color.
Expand All @@ -51,7 +52,7 @@ class CORE_EXPORT ColorChooser : public GarbageCollectedMixin {
// Call to close the UI.
virtual void EndChooser() {}
// Returns a root AXObject in the ColorChooser if it's available.
virtual AXObject* RootAXObject() = 0;
virtual AXObject* RootAXObject(Element* popup_owner) = 0;
};

} // namespace blink
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -78,8 +78,8 @@ void ColorChooserPopupUIController::EndChooser() {
CancelPopup();
}

AXObject* ColorChooserPopupUIController::RootAXObject() {
return popup_ ? popup_->RootAXObject() : nullptr;
AXObject* ColorChooserPopupUIController::RootAXObject(Element* popup_owner) {
return popup_ ? popup_->RootAXObject(popup_owner) : nullptr;
}

void ColorChooserPopupUIController::WriteDocument(SharedBuffer* data) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@ class CORE_EXPORT ColorChooserPopupUIController final

// ColorChooser functions
void EndChooser() override;
AXObject* RootAXObject() override;
AXObject* RootAXObject(Element* popup_owner) override;

// PagePopupClient functions:
void WriteDocument(SharedBuffer*) override;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,7 @@ void ColorChooserUIController::EndChooser() {
client_->DidEndChooser();
}

AXObject* ColorChooserUIController::RootAXObject() {
AXObject* ColorChooserUIController::RootAXObject(Element* popup_owner) {
return nullptr;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ class CORE_EXPORT ColorChooserUIController
// ColorChooser functions:
void SetSelectedColor(const Color&) final;
void EndChooser() override;
AXObject* RootAXObject() override;
AXObject* RootAXObject(Element* popup_owner) override;

// mojom::blink::ColorChooserClient functions:
void DidChooseColor(uint32_t color) final;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -292,7 +292,7 @@ Vector<mojom::blink::ColorSuggestionPtr> ColorInputType::Suggestions() const {
}

AXObject* ColorInputType::PopupRootAXObject() {
return chooser_ ? chooser_->RootAXObject() : nullptr;
return chooser_ ? chooser_->RootAXObject(&GetElement()) : nullptr;
}

ColorChooserClient* ColorInputType::GetColorChooserClient() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,7 @@
namespace blink {

class AXObject;
class Element;

struct DateTimeChooserParameters {
DISALLOW_NEW();
Expand Down Expand Up @@ -87,7 +88,7 @@ class CORE_EXPORT DateTimeChooser : public GarbageCollected<DateTimeChooser> {

virtual void EndChooser() = 0;
// Returns a root AXObject in the DateTimeChooser if it's available.
virtual AXObject* RootAXObject() = 0;
virtual AXObject* RootAXObject(Element* popup_owner) = 0;

virtual void Trace(Visitor* visitor) const {}
};
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -81,8 +81,8 @@ void DateTimeChooserImpl::EndChooser() {
frame_->View()->GetChromeClient()->ClosePagePopup(popup_);
}

AXObject* DateTimeChooserImpl::RootAXObject() {
return popup_ ? popup_->RootAXObject() : nullptr;
AXObject* DateTimeChooserImpl::RootAXObject(Element* popup_owner) {
return popup_ ? popup_->RootAXObject(popup_owner) : nullptr;
}

static String ValueToDateTimeString(double value, InputType::Type type) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@ class CORE_EXPORT DateTimeChooserImpl final : public DateTimeChooser,

// DateTimeChooser functions:
void EndChooser() override;
AXObject* RootAXObject() override;
AXObject* RootAXObject(Element* popup_owner) override;

void Trace(Visitor*) const override;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -156,7 +156,7 @@ void ExternalDateTimeChooser::EndChooser() {
client->DidEndChooser();
}

AXObject* ExternalDateTimeChooser::RootAXObject() {
AXObject* ExternalDateTimeChooser::RootAXObject(Element* popup_owner) {
return nullptr;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@ namespace blink {

class DateTimeChooserClient;
class LocalFrame;
class Element;

class CORE_EXPORT ExternalDateTimeChooser final : public DateTimeChooser {
public:
Expand All @@ -55,7 +56,7 @@ class CORE_EXPORT ExternalDateTimeChooser final : public DateTimeChooser {

// DateTimeChooser function:
void EndChooser() override;
AXObject* RootAXObject() override;
AXObject* RootAXObject(Element* popup_owner) override;

mojom::blink::DateTimeChooser& GetDateTimeChooser(LocalFrame* frame);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -645,7 +645,7 @@ void InternalPopupMenu::UpdateFromElement(UpdateReason) {
}

AXObject* InternalPopupMenu::PopupRootAXObject() const {
return popup_ ? popup_->RootAXObject() : nullptr;
return popup_ ? popup_->RootAXObject(owner_element_) : nullptr;
}

void InternalPopupMenu::Update(bool force_update) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -152,7 +152,7 @@ void PickerIndicatorElement::DetachLayoutTree(bool performing_reattach) {
}

AXObject* PickerIndicatorElement::PopupRootAXObject() const {
return chooser_ ? chooser_->RootAXObject() : nullptr;
return chooser_ ? chooser_->RootAXObject(&OwnerElement()) : nullptr;
}

bool PickerIndicatorElement::IsPickerIndicatorElement() const {
Expand Down
3 changes: 2 additions & 1 deletion third_party/blink/renderer/core/page/page_popup.h
Original file line number Diff line number Diff line change
Expand Up @@ -40,12 +40,13 @@ class Rect;
namespace blink {

class AXObject;
class Element;

// A PagePopup object is created by ChromeClient::openPagePopup(), and deleted
// by ChromeClient::closePagePopup().
class PagePopup {
public:
virtual AXObject* RootAXObject() = 0;
virtual AXObject* RootAXObject(Element* popup_owner) = 0;
virtual void SetWindowRect(const gfx::Rect&) = 0;
virtual void PostMessageToPopup(const String& message) = 0;
virtual void Update() = 0;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -328,9 +328,9 @@ class MODULES_EXPORT AXObjectCacheImpl
// be created. For instance, not all HTMLElements can have an AXObject,
// such as <head> or <script> tags.
AXObject* GetOrCreate(AccessibleNode*, AXObject* parent);
AXObject* GetOrCreate(LayoutObject*, AXObject* parent_if_known) override;
AXObject* GetOrCreate(LayoutObject*, AXObject* parent_if_known);
AXObject* GetOrCreate(LayoutObject* layout_object);
AXObject* GetOrCreate(const Node*, AXObject* parent_if_known);
AXObject* GetOrCreate(const Node*, AXObject* parent_if_known) override;
AXObject* GetOrCreate(Node*, AXObject* parent_if_known);
AXObject* GetOrCreate(Node*);
AXObject* GetOrCreate(const Node*);
Expand All @@ -346,7 +346,7 @@ class MODULES_EXPORT AXObjectCacheImpl
AXObject* Get(AbstractInlineTextBox*);

// Get an AXObject* backed by the passed-in DOM node.
AXObject* Get(const Node*);
AXObject* Get(const Node*) override;
// Get an AXObject* backed by the passed-in LayoutObject, or the
// LayoutObject's DOM node, if that is available.
// If |parent_for_repair| is provided, and the object had been detached from
Expand Down

0 comments on commit 9f8b3b4

Please sign in to comment.