[Mac] Do not expose HTML table semantics for "layout" tables

RESOLVED FIXED in Firefox 44

Status

()

RESOLVED FIXED
3 years ago
3 years ago

People

(Reporter: fredw, Assigned: fredw)

Tracking

Trunk
mozilla44
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox42 affected, firefox44 fixed)

Details

Attachments

(3 attachments, 3 obsolete attachments)

(Assignee)

Comment 1

3 years ago
Created attachment 8667313 [details]
Testcase
Assignee: nobody → fred.wang
Status: NEW → ASSIGNED
(Assignee)

Comment 3

3 years ago
Created attachment 8667522 [details]
Testcase
Attachment #8667313 - Attachment is obsolete: true
(Assignee)

Comment 4

3 years ago
Trying attachment 8667522 [details] with Yura, the layout tables are now indeed rendered with AXGroup objects.

However, this makes nighly crash in accessibilityAttributeValue with the "Attachments" of Bugzilla...
(Assignee)

Comment 5

3 years ago
Created attachment 8667580 [details]
bugzilla-table.html
(Assignee)

Updated

3 years ago
Depends on: 1210023
(Assignee)

Updated

3 years ago
Group: core-security
(Assignee)

Comment 6

3 years ago
> Group: core-security

Sorry, I didn't want to make this a security bug.
(Assignee)

Comment 7

3 years ago
@Marco Can you please try that one:

http://ftp.mozilla.org/pub/mozilla.org/firefox/try-builds/fred.wang@free.fr-b123129dfc3f/try-macosx64/firefox-44.0a1.en-US.mac.dmg

I verified that the crash no longer happens and that the table elements are now presented as AXGroups. However, I don't know if that's what you meant in bug 744790 comment 5.
Flags: needinfo?(mzehe)

Comment 8

3 years ago
Yes, this works like I would expect now.
Flags: needinfo?(mzehe)
(Assignee)

Updated

3 years ago
Attachment #8667402 - Flags: review?(surkov.alexander)
Comment on attachment 8667402 [details] [diff] [review]
Patch

Review of attachment 8667402 [details] [diff] [review]:
-----------------------------------------------------------------

::: accessible/mac/mozAccessible.mm
@@ +223,5 @@
>  
>    NS_OBJC_END_TRY_ABORT_BLOCK_RETURN(NO);
>  }
>  
> +- (BOOL)isComponentOfLayoutTable

nit: might be nice to name if isLayoutTablePart to make it sync with nsAccessibilityService names

@@ +241,5 @@
> +      table = accWrap->AsTableCell()->Table();
> +    }
> +    if (table && table->IsProbablyLayoutTable()) {
> +      return true;
> +    }

nit: return false; and no else

@@ +939,5 @@
>    }
>  
> +  if ([self isComponentOfLayoutTable]) {
> +    return NSAccessibilityGroupRole;
> +  }

I wish we had something nicer than checking table stuff for each role.
Attachment #8667402 - Flags: review?(surkov.alexander) → review+
(Assignee)

Comment 10

3 years ago
Created attachment 8668195 [details] [diff] [review]
Patch V2
Attachment #8667402 - Attachment is obsolete: true
(Assignee)

Comment 11

3 years ago
Created attachment 8668197 [details] [diff] [review]
Patch V3
Attachment #8668195 - Attachment is obsolete: true
(Assignee)

Updated

3 years ago
Whiteboard: [please remove from security-sensitive group]
(Assignee)

Comment 12

3 years ago
https://treeherder.mozilla.org/#/jobs?repo=try&revision=45bd8cd8985e

(In reply to alexander :surkov from comment #9)
> I wish we had something nicer than checking table stuff for each role.

This will be handled in bug 1178272.
(Assignee)

Comment 13

3 years ago
https://treeherder.mozilla.org/#/jobs?repo=try&revision=45bd8cd8985e
Keywords: checkin-needed
Whiteboard: [please remove from security-sensitive group] → [please remove from security-sensitive group - see comment 6]
(Assignee)

Updated

3 years ago
Blocks: 1178272

Updated

3 years ago
Group: core-security
Whiteboard: [please remove from security-sensitive group - see comment 6]
https://hg.mozilla.org/mozilla-central/rev/96f19a715e61
Status: ASSIGNED → RESOLVED
Last Resolved: 3 years ago
status-firefox44: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla44
You need to log in before you can comment on or make changes to this bug.