Closed Bug 1144516 Opened 7 years ago Closed 6 years ago
Reader] Wrong position of highlight box in content process' iframe member
When the screen reader focuses on the member of iframe of a child process, the highlight box will be drawn at wrong position on the screen while the system can still read the right element.
When focus changes the EventManager.jsm will get an AccEvent and pass the position of nsIAccessiblePivot to AccessFu.jsm by sending a "Access:Present" message. The AccessFu.jsm in parent process will adjust this position by adding the offset of window in parent process And set the bound to highlight box. And as my observation, the position of nsIAccessiblePivot is already incorrect which can be got from Accessible::Bounds() .  https://dxr.mozilla.org/mozilla-central/source/accessible/jsat/EventManager.jsm#149  https://dxr.mozilla.org/mozilla-central/source/accessible/jsat/EventManager.jsm#531  https://dxr.mozilla.org/mozilla-central/source/accessible/generic/Accessible.cpp#670
out of curiosity, what is AccessFu architecture for multiprocess?
(In reply to alexander :surkov from comment #2) > out of curiosity, what is AccessFu architecture for multiprocess? [cross-post from bug 1113494] Current ScreenReader Flow - [Gaia] use mozSettings “accessibility.screenreader” to start  - [Gecko-b2g] AccessFu.jsm inject content-script.js into all content processes - [Gecko-content] content-script.js initialize EventManager, ContentControl, and all required accessibility API - [Gecko-content] ContentControl capture mousemove events and move visual caret by vc.moveToPoint()  - [Gecko-content] handle moveToPoint, move visual cursor to corresponding element. - [Gecko-content] EventManager catch visual cursor change event and trigger Presentation.pivotChanged callback  - [Gecko-content] VisualPresenter.pivotChanged will create a corresponding pivot context ('vc-change') for moving highlighting indicator  - [Gecko-content] B2GPresenter.pivotChanged will create a corresponding pivot context (‘vc-change’) for speech synthesis  - [Gecko-content] EventManager send AccessFu:Present with both pivot contexts - [Gecko-b2g] AccessFu.jsm received AccessFu:Present message - [Gecko-b2g] use Output.Visual() to adjust highlighting indicator  - [Gecko-b2g] use Output.B2G() to forward speech synthesis data via mozChromeEvent  - [Gaia] capture mozChromeEvent(‘accessibility-action’) 'vc-change’ type. - [Gaia] build utterance and speech synthesis.  Text for speech synthesis are generated by UtteranceGenerator according to element types. You may customize the generation rules.  AccessFu uses PointerAdapter.jsm to capture touch gesture which overrides the input behavior. You might want to disable it.   https://github.com/mozilla-b2g/gaia/blob/master/apps/settings/js/panels/accessibility_screenreader/confirm_dialog.js#L23  https://dxr.mozilla.org/mozilla-central/source/accessible/jsat/ContentControl.jsm#142  https://dxr.mozilla.org/mozilla-central/source/accessible/jsat/EventManager.jsm#149  https://dxr.mozilla.org/mozilla-central/source/accessible/jsat/Presentation.jsm#180  https://dxr.mozilla.org/mozilla-central/source/accessible/jsat/Presentation.jsm#505  https://dxr.mozilla.org/mozilla-central/source/accessible/jsat/AccessFu.jsm#531  https://dxr.mozilla.org/mozilla-central/source/accessible/jsat/AccessFu.jsm#525  https://github.com/mozilla-b2g/gaia/blob/master/apps/system/js/accessibility.js#L291  https://dxr.mozilla.org/mozilla-central/source/accessible/jsat/OutputGenerator.jsm#304  https://dxr.mozilla.org/mozilla-central/source/accessible/jsat/AccessFu.jsm#128
When pass the pivot position from content process to b2g process , it will be wrapped into an object 'PivotContext' defined in 'Utils.jsm'. When getting the position from this object, it will be scaled with a value called 'getContentResolution'. I have done a experiment which generate a div and iframe with the same size and position contains a text with offset 0 to the top and left, and let the screen reader focus on these texts separately. In this case, I get the same value of position from Accessible::Bounds(), but the values go different when I get the position from the PivotContext in b2g. This is caused by the different getContentResolution in PivotContext, in this case value is 0.32653 for div and 1 for the iframe. This might cause the wrong position of the highlight box, and I am tracking the meaning of this value now. https://dxr.mozilla.org/mozilla-central/source/accessible/jsat/EventManager.jsm#149  https://dxr.mozilla.org/mozilla-central/source/accessible/jsat/Utils.jsm#675  https://dxr.mozilla.org/mozilla-central/source/accessible/jsat/Utils.jsm#311  https://dxr.mozilla.org/mozilla-central/source/accessible/jsat/AccessFu.jsm#552 
In current patch, I use the resolution of content's root window for every windows. I am not sure if it makes sense, but it dose solve the problem in current cases. I will keep tracing this, maybe there are some condition that I ignore.
There is another problem for Accessibility code. When receiving the 'scroll' event, the highlight box will repaint on the scroll event's target window. However, if the current highlight box is drawn in an not scrollable iframe embedded in another. And once you scroll the window, the highlight box will repaint in outer iframe because the scroll event's target is the outer window.  https://dxr.mozilla.org/mozilla-central/source/accessible/jsat/EventManager.jsm#111  https://dxr.mozilla.org/mozilla-central/source/accessible/jsat/Presentation.jsm#158
In this patch, I save the target window when VIRTUALCURSOR_CHANGED event occurred. So, when receive the scroll event, I use the saved target window above instead of scroll event's. For now, there is no side effect in my consideration, but maybe there are some events or variables can describe this situation better.
Comment on attachment 8582338 [details] [diff] [review] Fix the position of highlight box in iframe when scroll the window This problem will be discussed at Bug 1148313
Attachment #8582338 - Attachment is obsolete: true
Comment on attachment 8579963 [details] [diff] [review] Modify the resolution to fix the incorrect position of highlight box in iframe In this patch, we can fix the highlight box's position in some cases(when the resolution of the iframe in mozapp is 1). In other cases(when the resolution of the iframe in mozapp is not 1), we cannot verify the correctness of this patch, because, in current b2g code, this situation won't happen. The current resolution-changed functions(e.g, Pinch zoom, orientation) can only change the root layer's resolution. I've try the function nsIDOMWindowUtils::SetResolutionAndScaleTo to change the resolution manually, however, it seems just scale the layout in compositor thread only but doesn't trigger the reflow. So the positions of the element don't really match the visual layout. I am still trying how to set up the situation to verify this patch, but have no idea so far. Therefore, it may take some time.
The problmen I see is that the rectangle is draw below the position. It looks like a recent change to the accessibility bounds made this. In the past, the accessible bounds inside a child OOP iframe were relative to the child document, now it seems like we get true screen coords, which is good. So this patch removes the offset that we added to OOP content.
Attachment 8579963 [details] [diff] doesn't remedy the issue I see. Perhaps these are two separate bugs?
Looks like this was introduced in bug 1075670.
(In reply to Eitan Isaacson [:eeejay] from comment #11) > Attachment 8579963 [details] [diff] doesn't remedy the issue I see. Perhaps > these are two separate bugs? Thanks for the help. Yes, I think these maybe can be considered as two separate bugs. The problem I state in this bug is more related to the resolution. attachment 8592374 [details] [diff] [review] can fix the rectangle position shift cause by bug 1075670. But when browsing the frame with resolution is not 1, the rectangle is still in incorrect position. But after bug 1075670, my solution cannot work anymore, and it seems kind of changing the act of the bug I state. I need some time to figure it out.
Comment on attachment 8592374 [details] [diff] [review] Remove offset of mozbrowser iframe. This seems to be a fix we should land.
Attachment #8592374 - Flags: review?(yzenevich)
Comment on attachment 8592374 [details] [diff] [review] Remove offset of mozbrowser iframe. Review of attachment 8592374 [details] [diff] [review]: ----------------------------------------------------------------- Looks good, tests pass too.
Attachment #8592374 - Flags: review?(yzenevich) → review+
You need to log in before you can comment on or make changes to this bug.