Some accessible objects are not marked as offscreen

RESOLVED FIXED in mozilla18

Status

()

Core
Disability Access APIs
RESOLVED FIXED
6 years ago
6 years ago

People

(Reporter: Alex Kulikov, Ai Squared, Assigned: surkov)

Tracking

(Blocks: 1 bug)

Trunk
mozilla18
x86_64
Windows 7
Points:
---
Bug Flags:
in-testsuite +

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(4 attachments, 1 obsolete attachment)

(Reporter)

Description

6 years ago
Created attachment 637016 [details]
1.png

User Agent: Mozilla/5.0 (Windows NT 6.1; WOW64) AppleWebKit/536.5 (KHTML, like Gecko) Chrome/19.0.1084.56 Safari/536.5

Steps to reproduce:

Steps to reproduce:
1) Open http://www.aisquared.com/ and find Facebook social plugin at the bottom of the page;
2)Scroll the plugin content so some text is overlapped by the "ZoomText on Facebook" section;
3)Open AccProbe and find an overlapped text;
4)Check object state.


Actual results:

Overlapped text does not have STATE_SYSTEM_OFFSCREEN state (but it is not visible on the screen).


Expected results:

Overlapped text should have STATE_SYSTEM_OFFSCREEN state.
(Reporter)

Updated

6 years ago
Component: Untriaged → Disability Access

Updated

6 years ago
Component: Disability Access → Disability Access APIs
Product: Firefox → Core
QA Contact: untriaged → accessibility-apis

Comment 1

6 years ago
Is the state off-screen really for stuff that is overlapped by other layers? I thought off-screen was for items that are scrolled outside, not overlapped.
(Reporter)

Comment 2

6 years ago
(In reply to Marco Zehe (:MarcoZ) from comment #1)
> Is the state off-screen really for stuff that is overlapped by other layers?
> I thought off-screen was for items that are scrolled outside, not overlapped.
Objects become visible if I call IAccessible2::scrollTo(IA2_SCROLL_TYPE_TOP_LEFT) method, so I think they are scrolled out.
(Assignee)

Updated

6 years ago
Blocks: 640520
(Assignee)

Comment 3

6 years ago
Alex, I don't see role heading anymore (grand parent of 'ZoomText on Facebook' link). Did they change the web site? Do you have another testcase?
(Reporter)

Comment 4

6 years ago
(In reply to alexander :surkov from comment #3)
> Alex, I don't see role heading anymore (grand parent of 'ZoomText on
> Facebook' link). Did they change the web site? Do you have another testcase?
I don't think they've changed the site. I can still reproduce the issue: text that is overlapped by the "ZoomText on Facebook" section does not have STATE_SYSTEM_OFFSCREEN state.
(Assignee)

Comment 5

6 years ago
(In reply to Alex Kulikov, Ai Squared from comment #4)
> (In reply to alexander :surkov from comment #3)
> > Alex, I don't see role heading anymore (grand parent of 'ZoomText on
> > Facebook' link). Did they change the web site? Do you have another testcase?
> I don't think they've changed the site. I can still reproduce the issue:
> text that is overlapped by the "ZoomText on Facebook" section does not have
> STATE_SYSTEM_OFFSCREEN state.

which text? where can I locate it? I don't longer see the heading accessible that was selected on your screenshot.
(Reporter)

Comment 6

6 years ago
Created attachment 658058 [details]
screenshot that illustrates the issue
(Reporter)

Comment 7

6 years ago
(In reply to alexander :surkov from comment #5)
> (In reply to Alex Kulikov, Ai Squared from comment #4)
> > (In reply to alexander :surkov from comment #3)
> > > Alex, I don't see role heading anymore (grand parent of 'ZoomText on
> > > Facebook' link). Did they change the web site? Do you have another testcase?
> > I don't think they've changed the site. I can still reproduce the issue:
> > text that is overlapped by the "ZoomText on Facebook" section does not have
> > STATE_SYSTEM_OFFSCREEN state.
> 
> which text? where can I locate it? I don't longer see the heading accessible
> that was selected on your screenshot.
I`ve attached a new screenshot that shows the issue. Please let me know if you need any additional information.
(Assignee)

Comment 8

6 years ago
I can't reproduce, when those items are scrolled off then I see offscreen state. Did you try nightly build?
(Reporter)

Comment 9

6 years ago
Created attachment 659973 [details]
The same issue on the FF Nightly 18.0a1 (2012-09-10)
(Reporter)

Comment 10

6 years ago
(In reply to alexander :surkov from comment #8)
> I can't reproduce, when those items are scrolled off then I see offscreen
> state. Did you try nightly build?
Sorry for the late response. I can reproduce the issue with the latest FF nightly build. I've attached a new screenshot that shows the issue. Please let me know if you need any additional info.
(Assignee)

Comment 11

6 years ago
I can reproduce
Status: UNCONFIRMED → NEW
Ever confirmed: true
(Assignee)

Comment 12

6 years ago
Created attachment 662422 [details] [diff] [review]
patch
Assignee: nobody → surkov.alexander
Status: NEW → ASSIGNED
Attachment #662422 - Flags: review?(trev.saunders)
Attachment #662422 - Flags: feedback?(roc)
Comment on attachment 662422 [details] [diff] [review]
patch

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

::: accessible/src/generic/Accessible.cpp
@@ +636,5 @@
> +      nsRect frameRect(frame->GetOffsetTo(parentFrame), frame->GetSize());
> +      if (!scrollPortRect.Contains(frameRect)) {
> +        const nscoord kMinPixels = nsPresContext::CSSPixelsToAppUnits(12);
> +        scrollPortRect.Deflate(kMinPixels, kMinPixels);
> +        if (frameRect.YMost() <= scrollPortRect.y)

Can't you use if (scrollPortRect.Intersects(frameRect))?
Attachment #662422 - Flags: feedback?(roc) → feedback+
(Assignee)

Comment 14

6 years ago
Created attachment 662494 [details] [diff] [review]
patch2
Attachment #662422 - Attachment is obsolete: true
Attachment #662422 - Flags: review?(trev.saunders)
Attachment #662494 - Flags: review?(trev.saunders)
(Assignee)

Comment 15

6 years ago
(In reply to alexander :surkov from comment #14)
> Created attachment 662494 [details] [diff] [review]
> patch2

1) using Intersects as roc suggested
2) use calculate offset to avoid double parent chain traversal and assertion that frame and scrollable frame belongs different documents
Comment on attachment 662494 [details] [diff] [review]
patch2

>+      nsRect scrollPortRect = scrollableFrame->GetScrollPortRect();
>+      nsRect frameRect(framePos, frame->GetSize());
>+      if (!scrollPortRect.Contains(frameRect)) {
>+        const nscoord kMinPixels = nsPresContext::CSSPixelsToAppUnits(12);
>+        scrollPortRect.Deflate(kMinPixels, kMinPixels);
>+        if (!scrollPortRect.Intersects(frameRect))
>+          return states::OFFSCREEN;
>+      }
>+    }

So, if I read this correctly if you have an accessible whose frame or parent frame is less than 16px by 16px but completely within the scroll rect then its considered visible, and on screen, but if you have the same amount of a larger frame in the scroll rect then its off screen, that seems kind of inconsistant, but I'm not sure what to do about it

>+      gQueue.push(new addTabInvoker("about:blank", testBackgroundTab));
>+      gQueue.push(new loadURIInvoker(gDocURI, testScrolledOff));
c> 

I'm not exactly thrilled about the indirection of the invoker taking an argument of the function to call.  I think it makes it a little more work to see what the test does.
Attachment #662494 - Flags: review?(trev.saunders) → review+
(Assignee)

Comment 17

6 years ago
(In reply to Trevor Saunders (:tbsaunde) from comment #16)

> So, if I read this correctly if you have an accessible whose frame or parent
> frame is less than 16px by 16px but completely within the scroll rect then
> its considered visible, and on screen, but if you have the same amount of a
> larger frame in the scroll rect then its off screen, that seems kind of
> inconsistant, but I'm not sure what to do about it

agree, just preserving the logic.

> >+      gQueue.push(new addTabInvoker("about:blank", testBackgroundTab));
> >+      gQueue.push(new loadURIInvoker(gDocURI, testScrolledOff));
> c> 
> 
> I'm not exactly thrilled about the indirection of the invoker taking an
> argument of the function to call.  I think it makes it a little more work to
> see what the test does.

it's good for code generalization (it can be reused very easy) but I'd agree that the testing function should be a primary thing, the action function should be a secondary thing.
(Assignee)

Comment 18

6 years ago
but if you don't mind then I would go with what we have, I don't have good ideas how to make things nicer
(Assignee)

Comment 19

6 years ago
http://hg.mozilla.org/integration/mozilla-inbound/rev/880c0c02dcc0
Flags: in-testsuite+
Target Milestone: --- → mozilla18
https://hg.mozilla.org/mozilla-central/rev/880c0c02dcc0
Status: ASSIGNED → RESOLVED
Last Resolved: 6 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.