Closed Bug 466629 Opened 16 years ago Closed 16 years ago

AttributeChangedEvent not fired, or at least test is failing, with spell check mistakes on Linux

Categories

(Core :: Disability Access APIs, defect)

Product:
Core
Component:
Disability Access APIs
Platform:
x86
Linux
Type:
defect
Priority:
Not set
Severity:
normal

Tracking

()

Status:
RESOLVED FIXED

People

(Reporter: MarcoZ, Assigned: surkov)

References

Details

(Keywords: access, verified1.9.1)

Attachments

(2 files)

patch
(deleted), patch
MarcoZ
: review+
Details | Diff | Splinter Review
patch 1.9.1
(deleted), patch
Details | Diff | Splinter Review
The test for text attribute changes in the misspellings test of test_textattrs.html consistently fails on Linux. The event is not generated. As a result, Orca won't be able to pick up the fact that something just got misspelled.
It's nice but I have this on windows as well.
(In reply to comment #1) > It's nice but I have this on windows as well. though the first time only
I believe it's because document has busy state and we don't process selection (normal and spellcheck) changes in this case.
Initially state busy check has been added in bug 404380 to avoid crashes when caret move events are fired on startup. So possibly we should follow this way for spellcheck event. In this case all we need is to wait in mochitests when document will be loaded. Any opinions?
Sounds good Alex! If there's a way to fix this inside the Mochitest, go for it!
Attached patch patch (deleted) — — Splinter Review
Assignee: nobody → surkov.alexander
Status: NEW → ASSIGNED
Attachment #353216 - Flags: review?(marco.zehe)
Comment on attachment 353216 [details] [diff] [review] patch Good catch! We should add the call to AddA11YEventListener to all files to make sure we always run with the correct document's state.
Attachment #353216 - Flags: review?(marco.zehe) → review+
Comment on attachment 353216 [details] [diff] [review] patch Surkov, good idea to wait for a11y before testing :) Regarding: >+ var gTextAttrChangedEventHandler = { >+ handleEvent: function gTextAttrChangedEventHandler_handleEvent(aEvent) Why add "gTextAttrChangedEventHandler_handleEvent" here? Can we just use the simpler: handleEvent: function (aEvent)
(In reply to comment #8) > (From update of attachment 353216 [details] [diff] [review]) > Surkov, good idea to wait for a11y before testing :) Regarding: > > >+ var gTextAttrChangedEventHandler = { > >+ handleEvent: function gTextAttrChangedEventHandler_handleEvent(aEvent) > > Why add "gTextAttrChangedEventHandler_handleEvent" here? Can we just use the > simpler: > > handleEvent: function (aEvent) Yes, we could. But rules of good form assumes not to use anonymous functions. At least it makes hard to debug JS - the name 'anonymous' is used in functions stack.
http://hg.mozilla.org/mozilla-central/rev/9668585ab6a1
Status: ASSIGNED → RESOLVED
Closed: 16 years ago
Resolution: --- → FIXED
Alex, please create a mozilla-1.9.1 version of this Mochitest fix as well so we can get the Tinderboxes green on that tree for a11y as well.
Attached patch patch 1.9.1 (deleted) — — Splinter Review
Am I free to put this patch to 1.9.1 without any approval?
(In reply to comment #12) > Am I free to put this patch to 1.9.1 without any approval? Yes, test-only fixes can go in without approval AFAIK.
Pushed on Alexander's behalf in changeset: http://hg.mozilla.org/releases/mozilla-1.9.1/rev/d81250c33bb3
Keywords: fixed1.9.1
Test-only fix, ran on 1.9.1 tre since checkin on December 18, 2008, without problems.
Keywords: fixed1.9.1 → verified1.9.1
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: