Closed
Bug 1162434
Opened 9 years ago
Closed 9 years ago
Make ISimpleDOM work on X64
Categories
(Core :: Disability Access APIs, defect)
Tracking
()
RESOLVED
FIXED
mozilla41
Tracking | Status | |
---|---|---|
firefox38 | --- | unaffected |
firefox38.0.5 | --- | wontfix |
firefox39 | --- | wontfix |
firefox40 | --- | fixed |
firefox41 | --- | fixed |
People
(Reporter: MarcoZ, Assigned: surkov)
References
Details
Attachments
(2 files, 1 obsolete file)
1.13 KB,
patch
|
MarcoZ
:
review+
Sylvestre
:
approval-mozilla-aurora+
|
Details | Diff | Splinter Review |
1.29 KB,
patch
|
surkov
:
review+
Sylvestre
:
approval-mozilla-aurora+
|
Details | Diff | Splinter Review |
The SDN part also needs to use the new 32 bit IDs introduced in bug 606080. NVDA now works in 64 bit Firefox, JAWS doesn't. JAWS still uses ISimpleDOM heavily and thus needs to get the correct IDs.
Reporter | ||
Comment 1•9 years ago
|
||
Try push with a WIP patch: https://treeherder.mozilla.org/#/jobs?repo=try&revision=b38b22a215ef
Reporter | ||
Comment 2•9 years ago
|
||
Alex, any ideas? After the patch for bug 606080 landed, NVDA now works fine with 64 bit builds of nightly (or try server), but JAWS doesn't. It invokes its virtual buffer, but the only content it gets is the NetscapeHWND or whatever it calls it. I checked, and we seem to be firing the window emulation JAWS still relies on. So my hunch was to do the same as for the Unique IDs in IAccessible, for SDNAccessible. I worked up this patch, which compiles, but the result is the same. The virtual buffer seems to be invoked, but no content is being added. And debugging doesn't help me because the screen reader and browser run into a deadlock as soon as I try to run a local debug build. Same debug problems as usual.
Attachment #8602651 -
Flags: feedback?(surkov.alexander)
Reporter | ||
Comment 3•9 years ago
|
||
Opt Release build for the try run can be found here: http://ftp.mozilla.org/pub/mozilla.org/firefox/try-builds/mzehe@mozilla.com-b38b22a215ef/try-win64/
Assignee | ||
Comment 4•9 years ago
|
||
(In reply to Marco Zehe (:MarcoZ) from comment #2) > So my hunch was to do the same as for the Unique IDs in IAccessible, for > SDNAccessible. I worked up this patch, which compiles, but the result is the > same. perhaps you're right and they rely on uniqueID for DOM node (your patch makes it 0 when DOM node is not accessible). I'll check it out with them.
Reporter | ||
Comment 5•9 years ago
|
||
FYI, Window-Eyes is also not able to access the document. (In reply to alexander :surkov from comment #4) > perhaps you're right and they rely on uniqueID for DOM node (your patch > makes it 0 when DOM node is not accessible). I'll check it out with them. But if the DOM node is accessible, it should be correct, right? Or is this the totally wrong approach to the problem of getting the Unique ID for SDNAccessible?
Flags: needinfo?(surkov.alexander)
Assignee | ||
Comment 6•9 years ago
|
||
(In reply to Marco Zehe (:MarcoZ) from comment #5) > FYI, Window-Eyes is also not able to access the document. > > (In reply to alexander :surkov from comment #4) > > perhaps you're right and they rely on uniqueID for DOM node (your patch > > makes it 0 when DOM node is not accessible). I'll check it out with them. > > But if the DOM node is accessible, it should be correct, right? yes > Or is this > the totally wrong approach to the problem of getting the Unique ID for > SDNAccessible? I don't their internals, I asked them, let's see.
Flags: needinfo?(surkov.alexander)
Comment 7•9 years ago
|
||
(In reply to Marco Zehe (:MarcoZ) from comment #5) > FYI, Window-Eyes is also not able to access the document. > > (In reply to alexander :surkov from comment #4) > > perhaps you're right and they rely on uniqueID for DOM node (your patch > > makes it 0 when DOM node is not accessible). I'll check it out with them. > > But if the DOM node is accessible, it should be correct, right? Or is this > the totally wrong approach to the problem of getting the Unique ID for > SDNAccessible? they might also need a unique id from the UIA stuff which also needs to be fixed, but there I think you can just give them a 64 bit unique id.
Assignee | ||
Comment 8•9 years ago
|
||
Assignee: nobody → surkov.alexander
Attachment #8602651 -
Attachment is obsolete: true
Attachment #8602651 -
Flags: feedback?(surkov.alexander)
Attachment #8604238 -
Flags: review?(mzehe)
Reporter | ||
Comment 9•9 years ago
|
||
Comment on attachment 8604238 [details] [diff] [review] patch Yes, this fixes the problem. Thank you! (BTW: This is written with a local build that contains this patch, using JAWS.)
Attachment #8604238 -
Flags: review?(mzehe) → review+
Reporter | ||
Comment 10•9 years ago
|
||
Confirming that this also works with Window-Eyes.
Reporter | ||
Updated•9 years ago
|
Keywords: leave-open
Reporter | ||
Comment 12•9 years ago
|
||
Attachment #8605247 -
Flags: review?(surkov.alexander)
Assignee | ||
Comment 13•9 years ago
|
||
Comment on attachment 8605247 [details] [diff] [review] Part 2 - Fix ISimpleDOM Unique ID retrieval. Review of attachment 8605247 [details] [diff] [review]: ----------------------------------------------------------------- ::: accessible/windows/sdn/sdnAccessible.cpp @@ +106,5 @@ > // focus events, to correlate back to data nodes in their internal object > // model. > + AccessibleWrap* accessible = static_cast<AccessibleWrap*>(GetAccessible()); > + if (accessible) { > + *aUniqueID = AccessibleWrap::GetChildIDFor(accessible); GetChildIdFor takes Accessible*, you don't need to cast it to wrap
Attachment #8605247 -
Flags: review?(surkov.alexander) → review+
Reporter | ||
Updated•9 years ago
|
Keywords: leave-open
https://hg.mozilla.org/mozilla-central/rev/eb2201af1e2f
Status: NEW → RESOLVED
Closed: 9 years ago
status-firefox41:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla41
Reporter | ||
Comment 17•9 years ago
|
||
[Tracking Requested - why for this release]: If bug 606080 is permitted for 39, the patches in this bug need to land there, too. Setting "Affected" flag for that and requesting tracking.
status-firefox39:
--- → affected
tracking-firefox39:
--- → ?
Reporter | ||
Comment 18•9 years ago
|
||
Comment on attachment 8604238 [details] [diff] [review] patch Approval Request Comment [Feature/regressing bug #]: bug 606080 [User impact if declined]: Users of JAWS and Window-Eyes screen readers will not be able to use 64-bit editions of Firefox. JAWS is the market-leading screen reader. [Describe test coverage new/current, TreeHerder]: Manually tested on local and mozilla-central builds. [Risks and why]: Low ris,, this patch duplicated behavior from widget code needed for a call screen readers are making to a WndProc function. [String/UUID change made/needed]: None.
Attachment #8604238 -
Flags: approval-mozilla-aurora?
Reporter | ||
Comment 19•9 years ago
|
||
Comment on attachment 8605247 [details] [diff] [review] Part 2 - Fix ISimpleDOM Unique ID retrieval. Approval Request Comment [Feature/regressing bug #]: bug 606080 [User impact if declined]: Some functionality in commercial screen readers JAWS and Window-Eyes may be impaired. [Describe test coverage new/current, TreeHerder]: Tested on local build and mozilla-central build. [Risks and why]: Low risk, this just duplicates some code introduced in bug 606080 and uses a function introduced there on 64 bit systems. [String/UUID change made/needed]: None.
Attachment #8605247 -
Flags: approval-mozilla-aurora?
Updated•9 years ago
|
Attachment #8605247 -
Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Comment 20•9 years ago
|
||
Comment on attachment 8604238 [details] [diff] [review] patch Taking it as we care about accessibility
Attachment #8604238 -
Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Comment 21•9 years ago
|
||
https://hg.mozilla.org/releases/mozilla-aurora/rev/b3034590e03f https://hg.mozilla.org/releases/mozilla-aurora/rev/e8e924727dba
Comment 22•9 years ago
|
||
Marco, can you request uplift to beta also so we can get this fix into 39?
Flags: needinfo?(mzehe)
Comment 23•9 years ago
|
||
Wait, I just realized that is wrong, as we didn't take the fix for bug 606080 in 39. So I guess we have to wontfix this as well. I am marking 38.0.5 as wontfix assuming the win64 builds for 38.0.5 are also affected.
status-firefox38:
--- → unaffected
status-firefox38.0.5:
--- → wontfix
tracking-firefox39:
? → ---
Flags: needinfo?(mzehe)
Reporter | ||
Comment 24•9 years ago
|
||
Liz, yes, they are definitely affected. But as you said yourself, if we don't take bug 606080 for 39 after all, we won't need this one either. If that decision changes, however, we'll have to revisit this one, too.
You need to log in
before you can comment on or make changes to this bug.
Description
•