Position is not being updated when atk_text_set_caret_offset is used
Affects | Status | Importance | Assigned to | Milestone | |
---|---|---|---|---|---|
Mozilla Firefox |
Fix Released
|
High
|
|||
firefox (Ubuntu) |
Fix Released
|
Medium
|
Unassigned |
Bug Description
Binary package hint: firefox
This bug was filed last year upstream:
https:/
We have a working, tested patch, but more investigation is going into enhancing the new regression test case. This patch isn't in Firefox 3.6, or 3.7. However, this patch is critical for blind users who use Ubuntu with Orca. Without this patch, Firefox is basically unusable. If this patch could be included for Ubuntu Maverick, that would be great news for blind Ubuntu users.
ProblemType: Bug
DistroRelease: Ubuntu 10.04
Package: firefox 3.6.6+nobinonly
ProcVersionSign
Uname: Linux 2.6.32-23-generic x86_64
NonfreeKernelMo
Architecture: amd64
CrashDB: ubuntu
Date: Wed Jul 7 12:58:15 2010
FirefoxPackages:
firefox 3.6.6+nobinonly
firefox-
firefox-branding 3.6.6+nobinonly
abroswer N/A
abrowser-branding N/A
InstallationMedia: Vinux 3.0 RC4 X64 - Release amd64
ProcEnviron:
PATH=(custom, user)
LANG=en_US.UTF-8
SHELL=/bin/bash
SourcePackage: firefox
ThirdParty: True
In Mozilla Bugzilla #546068, Steve-holmes88 (steve-holmes88) wrote : | #2 |
Adding myself to CC list.
In Mozilla Bugzilla #546068, WaywardGeek (waywardgeek) wrote : | #3 |
I've verified the behavior Joanmarie describes with her test case. When I tab to the "First" link it is highlighted with a dotted line appears as a box around the link, and the caret is shown before the F in First. If I press the down arrow, the highlight on the "First" link goes away, and the caret is positioned before the S in "Second Heading". However, if I use the setCaretOffset() command to move, rather than the down button, I get a different result. The "First" link is still highlighted, but the caret is now shown at the start of the "Second Heading" line.
I believe the correct behavior of setCaretOffset() should cause the active link to become non-active.
In Mozilla Bugzilla #546068, WaywardGeek (waywardgeek) wrote : | #4 |
I have verified that this bug is caused by the cabb8925dcd3 commit, which refactors focus handling. Unfortunately, it modified over 100 files, so it's still not well narrowed down!
In Mozilla Bugzilla #546068, WaywardGeek (waywardgeek) wrote : | #5 |
Created an attachment (id=436749)
hg diff vs trunk - should work with patch
In Mozilla Bugzilla #546068, WaywardGeek (waywardgeek) wrote : | #6 |
I've located the problem, and think I understand it. It seems that TakeFocus() fails to work when called from setCaretOffsetCB, because it doesn't want to set to focus on things you can't tab to. I've added a flag to SetFocus called movedFocus, which basically forces the focus to be set. This is only done in calls from setCaretOffsetCB. Several files needed to be patched to deal with the new parameter, but the change really is that simple. One side effect is that the block of text being read by Orca is now highlighted with a box around it. I really like this. Apple does this with their screen reader as well.
In Mozilla Bugzilla #546068, WaywardGeek (waywardgeek) wrote : | #7 |
Created an attachment (id=436850)
Patch to fix Orca navigation
This patch can be applied against 40138:fa8b5b822730. The previous patch had a goober that's fixed in this one.
In Mozilla Bugzilla #546068, WaywardGeek (waywardgeek) wrote : | #8 |
Created an attachment (id=436913)
Patch to fix Orca navigation
This patch restricts changes to behavior to just calls to setCaretOffsetCB. The focus is forced, even though the caret may have moved to a non-tabbable element. This allows Orca's structural navigation to work with Firefox 3.6 and newer.
In Mozilla Bugzilla #546068, Bolterbugz (bolterbugz) wrote : | #9 |
Bill thanks very much for your patch! I am cc'ing some additional folks who should probably take a look. It isn't clear to me whether setting the focus is the right thing to do.
In Mozilla Bugzilla #546068, Alexander Surkov (surkov-alexander) wrote : | #10 |
(In reply to comment #9)
> I am cc'ing some additional folks who
> should probably take a look.
First of all I would like to get Enn's attention since this is mostly focus manager changes. That was a reason I've asked Enn for feedback on previous patch.
In Mozilla Bugzilla #546068, Marco-zehe (marco-zehe) wrote : | #11 |
(From update of attachment 436913)
Passing on review request to Alex for the accessibility part and Neil Deakin for the rest, since he wrote the original implementation. We'd also need a Mochitest based on the testcase that exercises this code.
In Mozilla Bugzilla #546068, Enn (enndeakin) wrote : | #12 |
It seems that setCaretOffset should actually just be calling nsFocusManager:
In Mozilla Bugzilla #546068, WaywardGeek (waywardgeek) wrote : | #13 |
I thought of that, and tried it. Something didn't work though. While the "First" link is unhighlighted, the tab key still causes a move to the "Second" link, rather than the link after the caret.
Sometime next week I can probably look into why the tab key moves us to the wrong link. Do you know what function I should put a breakpoint in to see what happens when I press tab? By the way, Neil, thank you for the feedback, and for the new focus manager. I'm hoping your work will allow us to get rid of a lot of hacks in Orca that were needed to deal with old Firefox focus/caret issues. I'm perfectly happy to do the debugging work if you're willing to review my patches and make suggestions.
In Mozilla Bugzilla #546068, Enn (enndeakin) wrote : | #14 |
The element to tab to is determined by nsFocusManager:
In Mozilla Bugzilla #546068, WaywardGeek (waywardgeek) wrote : | #15 |
Created an attachment (id=441149)
Patch based on using MoveFocus instead of TakeFocus
This patch works much better than the one where I forced the focus. The change is also localized to one function in nsHyperTextAcce
It is not clear to me if this change belongs in the SetSelectionRange function, or in SetCaretOffset, which simply calls SetSelectionRange. Since the call to TakeFocus in SetSelectionRange doesn't work on non-tabbabe elements, I reasoned that the change belongs there. However, I also see some versions of Firefox have removed the call to TakeFocus completely, which may make sense. I made my changes to changeset 41170:020a0670ef30, which is tagged "tip". If this patch looks like the right approach, I could use a pointer to how to Mozilla-fy it, and resubmit the patch.
In Mozilla Bugzilla #546068, Marco-zehe (marco-zehe) wrote : | #16 |
Bill, please ask review on the patch from the appropriate peers/module owners. Also, mark the other patch from april 4 as obsoleted so everybody knows which patch to work on. Thanks!
In Mozilla Bugzilla #546068, WaywardGeek (waywardgeek) wrote : | #17 |
Created an attachment (id=441434)
More properly formatted patch to fix set_caret_position
This is the same as the previous patch, but formatted properly. I've had time to do more user-testing, and I've made a similar patch to the Ubuntu Firefox 3.6.5 package and uploaded it for testing by Vinux users. This patch does seem to fix this bug.
In Mozilla Bugzilla #546068, Marco-zehe (marco-zehe) wrote : | #18 |
(From update of attachment 441434)
A few comments:
>+ /* Now that selection is done, move the focus to the selection. */
For single-line comments, please use // comment style.
>+ nsIDOMElement* aElement;
>+ fm->MoveFocus(
This is a local variable, not a parameter passed into this function. Therefore, it should not be aElement.
Requesting review from Surkov, but leaving review request for Neil Deakin intact for now for correct focusManager usage.
In Mozilla Bugzilla #546068, Alexander Surkov (surkov-alexander) wrote : | #19 |
(In reply to comment #15)
> Since the call to
> TakeFocus in SetSelectionRange doesn't work on non-tabbabe elements, I reasoned
> that the change belongs there.
I think TakeFocus() should work on focusable element, it shouldn't depend whether element is focusable or not.
> This patch works much better than the one where I forced the focus. The change
> is also localized to one function in nsHyperTextAcce
> several files. Basically, rather than TakeFocus, I used MoveFocus with the
> nsIFocusManager
> MoveFocus gets called when I move with arrow keys, and I called it the same
> way. It relies on there being a selection, so rather than calling it before
> setting the selection where we use to call TakeFocus, I call MoveFocus after
> selection.
What's happen when caret navigation mode is off?
Bill, could you please work on mochitests as well? Btw, does a11y mochitests pass?
In Mozilla Bugzilla #546068, WaywardGeek (waywardgeek) wrote : | #20 |
> For single-line comments, please use // comment style.
Oops! Sorry... I write a lot of C.
> This is a local variable, not a parameter passed into this function.
> Therefore, it should not be aElement.
I am confused abut the function of this parameter, and was trying to pass a dummy argument. However, elsewhere I see the parameter declared like this:
nsCOMPtr<
And passed with an expression like getter_
> I think TakeFocus() should work on focusable element, it shouldn't depend
> whether element is focusable or not.
I agree that TakeFocus should work on elements that aren't considered focusable. However, I have traced through the call to TakeFocus many times, and can confidently say that it does not take the focus on non-tabbable elements.
In layout/
isFocusable = mContent-
returns false, which causes TakeFocus to do nothing. My first patch that mostly worked was to add a force-focus flag to TakeFocus, which forced the focus onto unfocusable elements. The result was that paragraphs were highlighted with a box around them when navigating, which I quite like, but it is different behavior than we expect in Firefox. It is this inability for TakeFocus to focus on the unfocusable elements that causes this bug. Neil suggested MoveFocus, as that's what's used when moving the caret normally. I was able to confirm that Niel was correct, and I added a MoveFocus command that is similar to the one that causes the focus to change due to normal cursor movement.
> What's happen when caret navigation mode is off?
I assume you mean when press F7 to hide the caret, but still using Orca. It behaves exactly the same way, but I can't see the caret. The bug is still present. If you mean Orca Caret navigation off, the bug is also present with the same behavior. If you mean when Orca isn't running, I'm not able to reproduce the bug because I can't do structural navigation. However, when the cursor moves from a link into non-focusable text, the link is unhighlighed, and wherever the caret moves, if I press Tab, I go to the link after the caret. With Orca running, the link stays focused, and unless I highlight another link, Tab takes me to the link after the focused one. With the change to MoveFocus, the behavior becomes the same as when not running Orca.
> Bill, could you please work on mochitests as well? Btw, does a11y
> mochitests pass?
Sure, but first I'll have to figure out what mochitests are! I'm sure Google will show me the pages I need to read. I'll post back here when I find out what happens in mochitests, and if this patch causes no failures, I'll start working on a new mochitest for this bug.
In Mozilla Bugzilla #546068, Alexander Surkov (surkov-alexander) wrote : | #21 |
(In reply to comment #20)
> I agree that TakeFocus should work on elements that aren't considered
> focusable.
If TakeFocus would work as expected then perpahs you don't need to fix SetSelectionBounds.
> > Bill, could you please work on mochitests as well? Btw, does a11y
> > mochitests pass?
>
> Sure, but first I'll have to figure out what mochitests are! I'm sure Google
> will show me the pages I need to read. I'll post back here when I find out
> what happens in mochitests, and if this patch causes no failures, I'll start
> working on a new mochitest for this bug.
You will find our mochitests in accessible/
In Mozilla Bugzilla #546068, Alexander Surkov (surkov-alexander) wrote : | #22 |
Enn, how can we focus the focusable element? Currently we do fm->SetFocus(
In Mozilla Bugzilla #546068, Enn (enndeakin) wrote : | #23 |
(In reply to comment #22)
> Enn, how can we focus the focusable element? Currently we do
> fm->SetFocus(
> elements (please see comment #20 for details).
You don't.
Could you give an example which describes what you need to do?
In Mozilla Bugzilla #546068, Alexander Surkov (surkov-alexander) wrote : | #24 |
(In reply to comment #23)
> (In reply to comment #22)
> > Enn, how can we focus the focusable element? Currently we do
> > fm->SetFocus(
> > elements (please see comment #20 for details).
>
> You don't.
>
> Could you give an example which describes what you need to do?
I think focusable assumes it can be focused I think. AT should expect that. Other reason not-tabbable but focusable element could have keys processing like tabs in addons manager aren't tabbable but if the tab is focused (by click) then you can use right/left arrows to change panels. That's might be bad design. But since focused elements can have own keyboard processing and since it's very confusing focusable element what can't be focused then we need to fix it I think.
In Mozilla Bugzilla #546068, Enn (enndeakin) wrote : | #25 |
Are you asking about elements that can be focused but not in the tab order? That can be adjusted by setting the element's tabindex attribute.
Note that Mozilla will only ever fire key events at focused elements.
In Mozilla Bugzilla #546068, Alexander Surkov (surkov-alexander) wrote : | #26 |
(In reply to comment #25)
> Are you asking about elements that can be focused but not in the tab order?
> That can be adjusted by setting the element's tabindex attribute.
case by case?
> Note that Mozilla will only ever fire key events at focused elements.
If focusable element that is not in tab order is clicked then will it stay focused?
In Mozilla Bugzilla #546068, Bolterbugz (bolterbugz) wrote : | #27 |
(In reply to comment #26)
> (In reply to comment #25)
> > Note that Mozilla will only ever fire key events at focused elements.
>
> If focusable element that is not in tab order is clicked then will it stay
> focused?
Example:
<div tabindex="-1">I can be focused if you click on me</div>
Alexander, is this what you mean?
In Mozilla Bugzilla #546068, WaywardGeek (waywardgeek) wrote : | #28 |
I've run the mochitest-a11y tests, with "make -C objdir-ff-release mochitest-a11y", and I see that at least two tests fail in caret related code. There were 10 failures before my change, that I assume are just current problems unrelated to my work. I'm running this on revision 41318:77cb2107b06c.
I will track down why the other tests are failing, and that may give me some insight into what the right fix is. However, I have to table this work for two or three weeks, to work on the Vinux 3.0 release. The patch is being tested by Vinux users now, and so far they haven't found problems related to it, so we'll go with this for now, and I'll get back to this bug after the 3.0 release.
In Mozilla Bugzilla #546068, Alexander Surkov (surkov-alexander) wrote : | #29 |
(In reply to comment #27)
> > If focusable element that is not in tab order is clicked then will it stay
> > focused?
>
> Example:
> <div tabindex="-1">I can be focused if you click on me</div>
>
> Alexander, is this what you mean?
If it's not in tab order then yes. Or it could be XUL toolbarbutton I guess.
In Mozilla Bugzilla #546068, Enn (enndeakin) wrote : | #30 |
(In reply to comment #29)
> > Example:
> > <div tabindex="-1">I can be focused if you click on me</div>
> >
> > Alexander, is this what you mean?
>
> If it's not in tab order then yes. Or it could be XUL toolbarbutton I guess.
So what is your question? David's example is an element that can be focused but is skipped when pressing Tab to modify the focus. It can be focused via element.focus() or if mouse-clicking on it would do so on the platform.
Is your problem that accessibility wants to use tab navigation? No accessibility code is doing so currently that I can see. It would be calling nsIFocusManager
I do however see code using nsIFocusManager
In Mozilla Bugzilla #546068, Alexander Surkov (surkov-alexander) wrote : | #31 |
Enn, I must misread the bug. Sorry. The first question is:
Let the caret navigation mode is on, some focusable element is focused. We need to move the caret into heading (HTML h1 for example) and make document focused.
We do
1) nsIFocusManager
2) Change ranges of related nsISelection object.
I think existing code should work well for HTML input but it appears it doesn't work on rich text elements (like heading) when caret navigation mode is on. What should we do to get desired result?
The second question is: when nsIFocusManager
In Mozilla Bugzilla #546068, Enn (enndeakin) wrote : | #32 |
(In reply to comment #31)
> Let the caret navigation mode is on, some focusable element is focused. We need
> to move the caret into heading (HTML h1 for example) and make document focused.
Moving the caret and then using the suggestion I made in comment 12 will update the focus accordingly. The latest patch appears to do that.
> We do
> 1) nsIFocusManager
That will and should do nothing.
> 2) Change ranges of related nsISelection object.
>
> I think existing code should work well for HTML input but it appears it doesn't
> work on rich text elements (like heading) when caret navigation mode is on.
> What should we do to get desired result?
How does it not work?
I don't know how the specifics of how the caret is moved. I only know that the caret is moved and the focus updated to match (this is done in nsSelectMoveScr
> The second question is: when nsIFocusManager
> while focus is in another window then it seems the window containing HTML
> button is not focused. Is that expected?
The document.
In Mozilla Bugzilla #546068, Enn (enndeakin) wrote : | #33 |
(From update of attachment 441434)
>+ if (fm) {
>+ nsCOMPtr<
>+ nsCOMPtr<
>+ nsCOMPtr<
Do any of these need to be null-checked? If not, I would just get them in one line and not store them in comptrs.
>+ nsIDOMElement* aElement;
>+ fm->MoveFocus(
This will leak aElement. You do need to use a comptr here.
The other similar places where the focus is updated to the caret position also use nsIFocusManager
In Mozilla Bugzilla #546068, WaywardGeek (waywardgeek) wrote : | #34 |
Created an attachment (id=447784)
Modified patch based on Neil's feedback
Fixed memory leak as Neil suggested. I wasn't sure if all these objects would be valid, so instead of in-lining the access, I added checks after each. If someone understands the context for when this function will be called better, I could potentially remove checks and in-line some. In my light testing, there were no warnings generated by these checks. I still need to check out why two regressions failed. I'll work on that next.
In Mozilla Bugzilla #546068, WaywardGeek (waywardgeek) wrote : | #35 |
With the new patch, applied to 42554:88a6e0534e03 tip, there are no new failures in the a11y tests. The errors that do occur result in identical log summaries. I'll get to work on a new mochitest now.
In Mozilla Bugzilla #546068, WaywardGeek (waywardgeek) wrote : | #36 |
Created an attachment (id=448030)
Patch, including a new mochitest test case
I've added a mochitest test case in this patch. Otherwise, it is identical to the previous, which tries to incorporate feedback from Neil's last post. I've verified that this test case fails without the patch to nsHyperTextAcce
In Mozilla Bugzilla #546068, Attila Hammer (hammera) wrote : | #37 |
Dear developers,
Because if I known right, Firefox 3.6.4 version coming out next week, i have a question only:
Not possible to add Bill already doed caret navigation related patch if not have need another work with need doing this related patch? This modification is very important for visual impaired users.
Bill, in Vinux/vinux-lucid repository, the modifyed 3.6.3 Firefox package what patch use?
If my request is impossible now, I agree, because I am not a Mozilla developer, but lot of visual impaired users very wait this bug fix. In future, the final patch is committed in 3.6.x series, or only 3.7 branch?
Sorry my possible silly questions,
Attila
In Mozilla Bugzilla #546068, Alexander Surkov (surkov-alexander) wrote : | #38 |
The patch doesn't have proper reviews still, wasn't landed on trunk. Usual practice is when patch was landed on trunk and no regressions weren't found during some time then we request an approval to land it on branch. Sometimes approval request consideration takes significant time from mozilla drivers, thus it's nearly impossible to include it into ongoing firefox release I think.
In Mozilla Bugzilla #546068, Attila Hammer (hammera) wrote : | #39 |
Hy,
Thank you Alex the answer. I agree.
Attila
In Mozilla Bugzilla #546068, Alexander Surkov (surkov-alexander) wrote : | #40 |
(From update of attachment 448030)
>+ /* Now that selection is done, move the focus to the selection. */
>+ nsCOMPtr<
>+ if (fm) {
>+ nsCOMPtr<
>+ NS_ENSURE_
>+ nsCOMPtr<
>+ NS_ENSURE_TRUE(doc, NS_ERROR_FAILURE);
It's preferable to add GetDocumentNode() method to nsAccessNode and use it here
>+ <script type="applicati
>+ function testFocus(aID, aAcc, aSelectedBefore, aSelectedAfter)
>+ {
nit: you could merge aID and aAcc
>+ is(aAcc.selected, aSelectedBefore,
>+ "Wrong selected state before focus for ID " + aID + "!");
>+ document.
>+ is(aAcc.selected, aSelectedAfter,
>+ "Wrong seleccted state after focus for ID " + aID + "!");
>+ }
>+
>+ function doTest()
>+ {
>+ ///////
>+ // normal hyperlink
>+ var link4 = getAccessible(
>+ testFocus("link4", link4, false, true);
>+ heading1 = getAccessible(
>+ heading1.
>+ testFocus("link4", link4, false, true);
I'm not sure what you test here, it looks like you ensure caretOffset removes the focus from link. Can you describe please what you want to check here?
In Mozilla Bugzilla #546068, WaywardGeek (waywardgeek) wrote : | #41 |
Hi, Alexander. Yes, I am testing that caretOffset removes the focus from link4. Without this patch, Firefox leaves the focus on link4 while moving the caret to heading1, so the second time we call testFocus, the test fails.
With this patch in place, we've still found focus related issues in Firefox, so there's more work to do. For example, when the Firefox window is activated, there is often no focus event generated by Firefox to tell Orca what has the focus. It would be great to get this patch into the hands of the Orca guys so we could start tracking down the next set of bugs. In any case, I'll take a look at adding a GetDocumentNode method to nsAccessNode, and I can remove the aAcc parameter.
Thanks,
Bill
In Mozilla Bugzilla #546068, Alexander Surkov (surkov-alexander) wrote : | #42 |
(In reply to comment #41)
> Hi, Alexander. Yes, I am testing that caretOffset removes the focus from
> link4. Without this patch, Firefox leaves the focus on link4 while moving the
> caret to heading1, so the second time we call testFocus, the test fails.
>
> With this patch in place, we've still found focus related issues in Firefox, so
> there's more work to do. For example, when the Firefox window is activated,
> there is often no focus event generated by Firefox to tell Orca what has the
> focus. It would be great to get this patch into the hands of the Orca guys so
> we could start tracking down the next set of bugs.
Is "heading1" getting focus event when you set caret offset on it? If it is then I would like to get focus event testing as well (you could use eventQueue object from events.js).
In Mozilla Bugzilla #546068, WaywardGeek (waywardgeek) wrote : | #43 |
I'll have to look into this, but my guess is that in FF 3.5, yes, it got focus events, and that after the large focus manager rewrite, no it does not. I think this behaviour change is also the cause of some of the other issues we're seeing in FF, even with this patch. However, from the discussion above, it's not clear if it's a bug or not for "heading1" to receive focus events. That's why I didn't test for it, as the correct behaviour still sounds in dispute. I'll find time this week to verify all this.
In Mozilla Bugzilla #546068, Alexander Surkov (surkov-alexander) wrote : | #44 |
(From update of attachment 448030)
>+ /* Now that selection is done, move the focus to the selection. */
nit: use //
>+ test_set_
it makes sense to reuse test_text_
>+ <script type="applicati
>+ function testFocus(aID, aAcc, aSelectedBefore, aSelectedAfter)
>+ {
>+ is(aAcc.selected, aSelectedBefore,
>+ "Wrong selected state before focus for ID " + aID + "!");
I find useful to test nsIAccessibleTe
>+ document.
>+ is(aAcc.selected, aSelectedAfter,
>+ "Wrong seleccted state after focus for ID " + aID + "!");
>+ }
>+
>+ function doTest()
>+ {
>+ ///////
>+ // normal hyperlink
>+ var link4 = getAccessible(
>+ testFocus("link4", link4, false, true);
>+ heading1 = getAccessible(
>+ heading1.
>+ testFocus("link4", link4, false, true);
technically you could replace it (see test_text_
invoke function:
focus link4
set caret offset for heading
event seq:
caret change (expected, this.eventSeq)
focus (not expected, this.unexpected
check function:
caret offset for heading
link4 should stay focused
please rerequest review when comments are addressed
btw, thank you for working on this!
In Mozilla Bugzilla #546068, WaywardGeek (waywardgeek) wrote : | #45 |
Ok, I'll try and update the patch to address these comments. I'll also investigate the old and new focus behaviour. One thing I'm fairly confident of however, is that after setting the caret offset in heading1, the test4 link should not (and does not after the patch) have the focus. Where the focus actually goes, I don't know! But, I'll find out.
WaywardGeek (waywardgeek) wrote : | #46 |
Binary package hint: firefox
This bug was filed last year upstream:
https:/
We have a working, tested patch, but more investigation is going into enhancing the new regression test case. This patch isn't in Firefox 3.6, or 3.7. However, this patch is critical for blind users who use Ubuntu with Orca. Without this patch, Firefox is basically unusable. If this patch could be included for Ubuntu Maverick, that would be great news for blind Ubuntu users.
ProblemType: Bug
DistroRelease: Ubuntu 10.04
Package: firefox 3.6.6+nobinonly
ProcVersionSign
Uname: Linux 2.6.32-23-generic x86_64
NonfreeKernelMo
Architecture: amd64
CrashDB: ubuntu
Date: Wed Jul 7 12:58:15 2010
FirefoxPackages:
firefox 3.6.6+nobinonly
firefox-
firefox-branding 3.6.6+nobinonly
abroswer N/A
abrowser-branding N/A
InstallationMedia: Vinux 3.0 RC4 X64 - Release amd64
ProcEnviron:
PATH=(custom, user)
LANG=en_US.UTF-8
SHELL=/bin/bash
SourcePackage: firefox
ThirdParty: True
WaywardGeek (waywardgeek) wrote : | #47 |
- Dependencies.txt Edit (3.4 KiB, text/plain; charset="utf-8")
- ExtensionSummary.txt Edit (842 bytes, text/plain; charset="utf-8")
- profile_default_pluginreg.dat.txt Edit (5.8 KiB, text/plain; charset="utf-8")
- profiles.ini.txt Edit (94 bytes, text/plain; charset="utf-8")
- This patch fixes the set_caret_offset function, and has a test case Edit (5.1 KiB, text/plain)
tags: | added: patch |
Attila Hammer (hammera) wrote : | #48 |
Bill, I confirming this problem with Ubuntu Lucid. Not possible apply with final ready patch not only Maverick, but Lucid and Maverick new later awailable Firefox update?
Attila
Attila
Changed in firefox (Ubuntu): | |
status: | New → Confirmed |
WaywardGeek (waywardgeek) wrote : | #49 |
Yes, it would be very nice to have this patch applied to future versions of Firefox for Lucid, as well as Maverick. This would simplify supporting Firefox in Vinux 3.0, which is based on Ubuntu Lucid, since we have to re-patch Firefox every time a new version is available.
Micah Gersten (micahg) wrote : | #50 |
Thank you for reporting this to Ubuntu. I've linked the upstream bug and will follow its progress. Once it lands on trunk upstream, I will try to get this landed on the 3.6 branch. We cannot take this patch w/out upstream approval. Please report any other issues you may find.
Changed in firefox (Ubuntu): | |
importance: | Undecided → Medium |
status: | Confirmed → Triaged |
Changed in firefox: | |
status: | Unknown → Confirmed |
In Mozilla Bugzilla #546068, Joanmarie (joanmarie-diggs-deactivatedaccount) wrote : | #51 |
(In reply to comment #45)
> Ok, I'll try and update the patch to address these comments.
Bill, any progress on this? As you know, this bug keeps biting us (Orca community) in the arse. :-( It would be beyond awesome if we could get it resolved once and for all. Thanks for your work!
tags: |
added: patch-forwarded-upstream removed: patch |
In Mozilla Bugzilla #546068, Joanmarie (joanmarie-diggs-deactivatedaccount) wrote : | #52 |
Ping?
Changed in firefox: | |
importance: | Unknown → High |
In Mozilla Bugzilla #546068, WaywardGeek (waywardgeek) wrote : | #53 |
Just tested this issue in Ubuntu Maverick, and it's still there. Isn't there someone at Mozilla who recently has been tasked to enhance gecko accessibility? Here would be an excellent place to start. I've had some health issues, and haven't been involved since September...
In Mozilla Bugzilla #546068, Alexander Surkov (surkov-alexander) wrote : | #54 |
Bill, thanks for bringing an attention to this, again. We have technical description of the problem and have a patch. It should be easy enough to finish it.
Fernando, wanna take a look?
In Mozilla Bugzilla #546068, Bolterbugz (bolterbugz) wrote : | #55 |
Approved blocking. Ubuntu bug says "Without this patch, Firefox is basically unusable". Making Fernando the owner at least for now. Bill, thanks again for all your work here.
In Mozilla Bugzilla #546068, Fernando Herrera (fherrera) wrote : | #56 |
Created attachment 494926
patch2
I have updated the patch moving the test case to test_text_
I also added code to remove all selections before moving the caret, it makes sense because that is what happens when moving it with the cursor (also is the way to get the caret-moved event fired: updating the selections).
Also, getCaretOffset was failing after a setCaretOffset because of the check "No caret if the focused node is not inside this DOM node and this DOM node is not inside of focused node". For that I'm just doing gLastFocusedNode = GetNode() when setting the caret, but I'm not sure about that solution.
Notice that tests are failing to check that link4 is still focused (MoveFocus will clear the focus).
In Mozilla Bugzilla #546068, Fernando Herrera (fherrera) wrote : | #57 |
Before I lost this patch surkov and I did some time ago, I'm attaching it.
The idea is to clear the focus on every blur event, cleaning our global gLastFocusedNode. And for nsHyperTextAcce
However we would need to rework how popup/menu end events are fired.
In Mozilla Bugzilla #546068, Fernando Herrera (fherrera) wrote : | #58 |
Created attachment 506135
Idea
In Mozilla Bugzilla #546068, Bolterbugz (bolterbugz) wrote : | #59 |
(In reply to comment #52)
> However we would need to rework how popup/menu end events are fired.
Got a plan? :)
In Mozilla Bugzilla #546068, Alexander Surkov (surkov-alexander) wrote : | #60 |
make sure to fix regressions in fx5
In Mozilla Bugzilla #546068, Alexander Surkov (surkov-alexander) wrote : | #61 |
(In reply to comment #52)
> Before I lost this patch surkov and I did some time ago, I'm attaching it.
>
> The idea is to clear the focus on every blur event, cleaning our global
> gLastFocusedNode. And for nsHyperTextAcce
> the ClearFocus().
>
> However we would need to rework how popup/menu end events are fired.
Honestly I don't recall my complicity in this idea development for this bug and failed to see how it works to make this bug fixed. Maybe it's indented for something else, I'm not sure.
In Mozilla Bugzilla #546068, Bolterbugz (bolterbugz) wrote : | #62 |
Cc'ing Trevor for serendipity.
Fernando, a friendly ping. Can you address comment 56 when you get a chance?
In Mozilla Bugzilla #546068, Alexander Surkov (surkov-alexander) wrote : | #63 |
(In reply to comment #56)
> (In reply to comment #52)
> > Before I lost this patch surkov and I did some time ago, I'm attaching it.
> >
> > The idea is to clear the focus on every blur event, cleaning our global
> > gLastFocusedNode. And for nsHyperTextAcce
> > the ClearFocus().
> >
> > However we would need to rework how popup/menu end events are fired.
>
> Honestly I don't recall my complicity in this idea development for this bug
> and failed to see how it works to make this bug fixed. Maybe it's indented
> for something else, I'm not sure.
Got it, maybe that was *this* bug that we talked about this solution for since this makes sure we fire focus event for document accessible when caret moves to non focusable content. Though we should deal with it separately since this bug can be reproduced easily out of this bug context (for example, shift tab from focused first focusable element).
In Mozilla Bugzilla #546068, Alexander Surkov (surkov-alexander) wrote : | #64 |
Created attachment 561436
patch3
In Mozilla Bugzilla #546068, Alexander Surkov (surkov-alexander) wrote : | #65 |
Created attachment 561438
patch4
correct patch
In Mozilla Bugzilla #546068, Marco-zehe (marco-zehe) wrote : | #66 |
Comment on attachment 561438
patch4
>+ //gQueue.push(new setCaretOffsetI
Why is this currently commented out?
And you need caret browsing enabled to see caretmove events, right?
In Mozilla Bugzilla #546068, Alexander Surkov (surkov-alexander) wrote : | #67 |
(In reply to Marco Zehe (:MarcoZ) from comment #61)
> Comment on attachment 561438
> patch4
>
> >+ //gQueue.push(new setCaretOffsetI
>
> Why is this currently commented out?
testing artifact
> And you need caret browsing enabled to see caretmove events, right?
yes
In Mozilla Bugzilla #546068, Marco-zehe (marco-zehe) wrote : | #68 |
Comment on attachment 561438
patch4
r=me.
In Mozilla Bugzilla #546068, Enn (enndeakin) wrote : | #69 |
Comment on attachment 561438
patch4
Not sure what you're asking me to review but the call to MoveFocus looks ok.
The FLAG_BYMOVEFOCUS won't flag actually do anything here.
In Mozilla Bugzilla #546068, Alexander Surkov (surkov-alexander) wrote : | #70 |
(In reply to Neil Deakin from comment #64)
> Comment on attachment 561438
> patch4
>
> Not sure what you're asking me to review but the call to MoveFocus looks ok.
>
> The FLAG_BYMOVEFOCUS won't flag actually do anything here.
Yes, for flags. I removed frameSelection-
Changed in firefox: | |
status: | Confirmed → In Progress |
In Mozilla Bugzilla #546068, Alexander Surkov (surkov-alexander) wrote : | #71 |
inbound land https:/
In Mozilla Bugzilla #546068, Ehsan-mozilla (ehsan-mozilla) wrote : | #72 |
Changed in firefox: | |
status: | In Progress → Fix Released |
In Mozilla Bugzilla #546068, Attila Hammer (hammera) wrote : Re: [Bug 602877] | #73 |
The committed fix will be awailable in Firefox 3.6 for example with
Lucid release future?
Attila
In Mozilla Bugzilla #546068, Alexander Surkov (surkov-alexander) wrote : | #77 |
(In reply to Launchpad from comment #68)
> The committed fix will be awailable in Firefox 3.6 for example with
> Lucid release future?
No, it'll be available for Firefox 10. It's tricky to port it on earlier releases because it's based on number of other fixes which are likely not going be ported too.
Micah Gersten (micahg) wrote : | #74 |
So, while this won't land in Firefox 3.6, we still need to figure out what to do with the LTS and Rapid Release, so it's likely that Lucid will get the fix at some point in the form of an upgrade.
Attila Hammer (hammera) wrote : Re: [Bug 602877] Re: Position is not being updated when atk_text_set_caret_offset is used | #76 |
Hy Micah,
Sorry if my comment parts are perhaps little offtopic this problem related:
This is a difficult question, because newest Firefox and Thunderbird
releases are containing for example following important bug for A11y
related:
https:/
A screen reader user important to hear the deleted characters for
example if writing a new e-mail or writing any data with a webpage, for
example if writing comments, or if online doing financial operations
with a net bank.
I using Ubuntu releases with screen reader support only, and Using
basicaly Firefox 3.6 release my Lucid system.
Since Firefox 4.0 release this bug is present, the developers working
this bug fixing, now I think this bug fix target milestone is Firefox 10.
Lucid release default containing Orca screen reader 2.30 version, with
not support I think default good with Thunderbird 3.1.x releases,
default slow the list navigation with large mail folders.
This problem are wonderful fixed with Orca 3.0 version, but this version
are default GNOME 3.0 compatible, I need doing a patched version of 3.0
package my Lucid system with not containing gsettings related changes.
Orca 3.0 release wonderful usable for example Thunderbird 3.1.15
version, but newest Thunderbird 7.0 release containing too this
backspace echoing related issue.
Newest Thunderbird 7.0 and Orca 3.2 release are have performance issues
for example with Oneiric, with I not found yet a workaround to resolve:
https:/
I no and agree, Mozilla support modell is changed, fastest releasing the
Mozilla products releases, so Ubuntu Mozilla team work is not easy to
always follow up changes. More sighted users using worldwide Ubuntu
releases, and would like upgrade perhaps the newest releases. But what
will be happening if an upgrade breaks A11y support for a Mozilla
product with visual impaired users worldwide?
Possible doing future an easy possibility to easy install prewious
Mozilla products version?
For example, if I install direct firefox-3.6 package version, will be
upgrading now automaticaly with newest upstream main releases with
rolling if main stable version are changing, because main firefox
package version is changing. For exampe, if you are uploading Lucid
branch the Firefox 7.0 package version, will be upgrading now my Firefox
version from 3.6 to 7.0, and I will be experiencing Orca not spokening
for example text entry fields deleted characters and very surprised if I
not known this A11y related bug.
Now not have possibility to I determine for example I would like only
Firefox 3.6 related security updates, or I don't known the good
technique. :-):-)
Not good possibility to visual impaired users holding Firefox and
Thunderbird releases, because this operation result not awailable the
security updates for a Firefox and Thunderbird release entire.
Lot of visual impaired users using Ubuntu or Ubuntu based accessibility
support completed derivative distribution.
An international distribution with affecting this upgrade is Vinux:
http://
Attila
Micah Gersten (micahg) wrote : | #78 |
This was fixed with Firefox 10 over a month ago.
Changed in firefox (Ubuntu): | |
status: | Triaged → Fix Released |
This sounds like another regression from bug 178324, similar to bug 542313.