Closed Bug 691734 Opened 8 years ago Closed 8 years ago

After bug 673958, opening a page with a defined anchor target in the address no longer causes NVDA to jump directly to that anchor.

Categories

(Core :: Disability Access APIs, defect)

x86_64
Windows 7
defect
Not set

Tracking

()

VERIFIED FIXED
mozilla10

People

(Reporter: MarcoZ, Assigned: surkov)

References

(Blocks 1 open bug, )

Details

(Keywords: regression)

Attachments

(1 file, 1 obsolete file)

After the big focus rework bug 673958, opening a page like the one provided in the URL above no longer causes NvDA to directly jump to the anchor target and start reading from there. Instead, virtual cursor always starts at the top. Before the build bug 673958 became checked into, 2009-09-28, this was the case, with bug 673958 in, it no longer is. Jumping from a link to an anchor target via link activation works fine, though. So I can jump from the table of content to any of the anchor targets on the same page. Only if it is opened from the URL bar and initially has an anchor target specified, it does not work.
Blocks: a11ynext
Similar to bug 628673. This could be because the scrollingStart event is fired before the focus event or it could just be a change in timing. If the former, it should be fixed. If the latter, we need bug 617544.
Testing with AccProbe, it looks like scrollingStart is being fired before the focus event, so this probably should be fixed.
(In reply to James Teh [:Jamie] from comment #1)
> Similar to bug 628673. This could be because the scrollingStart event is
> fired before the focus event or it could just be a change in timing. If the
> former, it should be fixed. If the latter, we need bug 617544.

Don't we want a bug 617544 for everything? Is the point that scrollingStart before focus event makes no sense? Taking care about events ordering should require us to introduce new not trivial code hunks.
(In reply to alexander surkov from comment #3)
> Don't we want a bug 617544 for everything?
We do definitely want that regardless, yes.

> Is the point that scrollingStart
> before focus event makes no sense?
Correct: I don't think scrollingStart before focus makes sense. If bug 617544 is fixed, this won't matter any more. However, I still don't think it makes sense. :)
Ok, I think we could implement IA2 proposal on Gecko side internally (i.e. implement functionality not IA2 interfaces) and fire scrollingStart event whenever documents gets focus and it has anchor target and then null out the anchor target if something else within a document gets the focus.
Attached patch patch (obsolete) — Splinter Review
Assignee: nobody → surkov.alexander
Status: NEW → ASSIGNED
Attachment #564843 - Flags: review?(marco.zehe)
Comment on attachment 564843 [details] [diff] [review]
patch

r- because this try-server build does not fix the problem for me at all. The behavior is just as observed with a regular nightly build. Jamie, can you see if you get any different events now than with a regular nightly?
Attachment #564843 - Flags: review?(marco.zehe) → review-
I think the answer is in bug 628673 comment #9:

> NVDA starts to read the page from beginning anyway. Do you create vb
> synchronously when you handle document load event or store events until vb is
> created after you scheduled its creation?
Ack. I forgot we create it asynchronously after receiving the event and don't store events for the buffer. The theory is that generally, we don't need any events for the buffer until the buffer is actually created. This is very much a timing issue. This is another reason for needing bug 617544: there shouldn't be any event state that can't be determined by making a query. I guess it must just be working for Marco in 3.6 because the timing is different.

We might be able to hack around this in NVDA, but honestly, I think it might just be worth waiting until bug 617544 can be done, which I imagine will be after FF4. The reason is that even if this is fixed, it'll still break if you move focus out of the document while it is loading. Also, this rarely works for me even in Firefox 3.6, probably because of the above timing issue.

It's probably still more correct to fire this event after the document loses the busy state.
(In reply to Marco Zehe (:MarcoZ) from comment #8)
> Jamie, can you
> see if you get any different events now than with a regular nightly?
I can confirm that the events are now fired in the correct order.

(In reply to alexander surkov from comment #9)
> I think the answer is in bug 628673 comment #9:
Correct. This is now purely a timing issue.

Recent changes in NVDA mean that I might be able to make this behave a little better now. In short, it's now possible to catch specific events even before the buffer is rendered. However, this still doesn't solve the problem of a document that finishes loading before NVDA even knows about it; e.g. loading a document before NVDA is started or restarting NVDA after a document has been loaded. For that, we need bug 617544.
(In reply to James Teh [:Jamie] from comment #10)
> However, this still doesn't solve the problem
> of a document that finishes loading before NVDA even knows about it; e.g.
> loading a document before NVDA is started or restarting NVDA after a
> document has been loaded.
I read the patch and it looked like scrollingStart gets fired on the anchor whenever the document gets focus. Subsequent testing confirms this. While this does actually fix the problem I mentioned, it causes a rather severe regression. Because scrollingStart gets fired every time focus moves away from and returns to the document, the user will keep losing their position in the document, as NVDA moves the cursor in response to scrollingStart events.
The ideal solution would be to only fire the initial scrollingStart event when the document *first* gains focus; i.e. not on every focus event.
I can fix it. But technically what is requirement to scrolling start event? Should it be fired after focus or after document is loaded?
(In reply to alexander surkov from comment #13)
> what is requirement to scrolling start event?
> Should it be fired after focus or after document is loaded?
"Loaded" is now a fairly vague term and it means nothing to NVDA now anyway. The best we can say is after focus.
Attached patch patch2Splinter Review
Attachment #564843 - Attachment is obsolete: true
Attachment #565154 - Flags: review?(marco.zehe)
Comment on attachment 565154 [details] [diff] [review]
patch2

r=me with one nit/question:
>+    const CC = Components.classes;

Where do you actually use this const? I don't see it in the file. A left-over?
Attachment #565154 - Flags: review?(marco.zehe) → review+
(In reply to Marco Zehe (:MarcoZ) from comment #16)
> Comment on attachment 565154 [details] [diff] [review] [diff] [details] [review]
> patch2
> 
> r=me with one nit/question:
> >+    const CC = Components.classes;
> 
> Where do you actually use this const? I don't see it in the file. A
> left-over?

that goes from tabbrowser bindings, it relies on variables and constants outside it
https://hg.mozilla.org/mozilla-central/rev/c664f85010b0
Status: ASSIGNED → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla10
Flags: in-testsuite+
Duplicate of this bug: 628673
Verified fixed in Mozilla/5.0 (Windows NT 6.1; WOW64; rv:10.0a1) Gecko/20111010 Firefox/10.0a1.

FYI, with this bug and NVDA main r4719, NVDA will always move to the right position as long as the document is loaded while NVDA is running. We still need bug 617544 to fix the case where a document is loaded before NVDA is started (or restarted).
Status: RESOLVED → VERIFIED
Depends on: 693658
Depends on: 591639
You need to log in before you can comment on or make changes to this bug.