Closed Bug 372367 Opened 15 years ago Closed 15 years ago

nsIAccessible::isEditable is dead

Categories

(Core :: Disability Access APIs, defect)

defect
Not set
normal

Tracking

()

RESOLVED FIXED

People

(Reporter: surkov, Assigned: surkov)

References

Details

Attachments

(2 files, 2 obsolete files)

Bug 340829 killed doc accessible implementation of isEditable for nsDocAccessible but it is still used in nsLinkableAccessible.

1) We can kill isEditable at all since it isn't used at all excepting of nsLinkableAccessible and use nsDocAccessible::GetState() & STATE_READONLY check instead.
2) Rehabilitate isEditable for nsDocAccessible.

Also nsDocAccessible::isEditable checked for states of editor:
    mEditor->GetFlags(&flags);
    *aIsEditable = (flags & nsIPlaintextEditor::eEditorReadonlyMask) == 0;

but nsDocAccessible::state check for editor itself:
  nsCOMPtr<nsIEditor> editor = GetEditor();
  if (!editor) {
    *aState |= STATE_READONLY;
  }

Is the check of nsDocAccessible::state enough for us?
Attached patch patch (obsolete) — Splinter Review
Attachment #258091 - Flags: review?(aaronleventhal)
Status: NEW → ASSIGNED
Comment on attachment 258091 [details] [diff] [review]
patch

Isn't it faster to specifically support nsIAccessibleEditableText in nsHTMLTextFieldAccessible? The "smart QI" impl in nsHyperTextAccessible requires it to look at the accessible children of the textfield.

But I don't really know if either is that much better.
Attachment #258091 - Flags: review?(aaronleventhal) → review+
(In reply to comment #2)
> (From update of attachment 258091 [details] [diff] [review])
> Isn't it faster to specifically support nsIAccessibleEditableText in
> nsHTMLTextFieldAccessible? The "smart QI" impl in nsHyperTextAccessible
> requires it to look at the accessible children of the textfield.

No, it checks whether mEditor is editable. I changed this because you said on irc nsIAccessibleEditableText should be exposed only if state is not readonly IIRC :).
Makes sense, actually. Go ahead and check in.

One thing -- supposedly according to rules of COM, the interfaces an object supports should remain the same throughout its lifetime. We probably break that rule if something changes its readonly property.
checked in

(In reply to comment #4)

> One thing -- supposedly according to rules of COM, the interfaces an object
> supports should remain the same throughout its lifetime. We probably break that
> rule if something changes its readonly property.
> 

Looks reasonable. Once I got nsIAccessibleEditableText then I can try to use it even if the current state is readonly.
Status: ASSIGNED → RESOLVED
Closed: 15 years ago
Resolution: --- → FIXED
patch backed out due to a regression,
firefox crashes when clicking links in webpages.
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Attached file core stack
it's an infinite recursion when GetState.
Damn. Apparently bug 264599 will fix this problem. And it should be right fix. For sure we can reintroduce again isEditable method and this will fix this bug too. Though it's not nice solution I guess. Let me look whether bug 264599 can be fixed easy enough.
I meant bug 344674.
Attached patch patch2 (obsolete) — Splinter Review
Attachment #258091 - Attachment is obsolete: true
Attachment #260357 - Flags: review?(ginn.chen)
Status: REOPENED → ASSIGNED
Attached patch patch3Splinter Review
Attachment #260357 - Attachment is obsolete: true
Attachment #260358 - Flags: review?(ginn.chen)
Attachment #260357 - Flags: review?(ginn.chen)
Attachment #260358 - Flags: review?(ginn.chen) → review+
checked in
Status: ASSIGNED → RESOLVED
Closed: 15 years ago15 years ago
Resolution: --- → FIXED
This is consistently causing crashes at startup for me when Window-Eyes is running. I'm not sure if it's the only thing causing bug 376239, but it's one cause.

Backing out until Surkov can look at it.
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
I was wrong, it looked like it fixed the problem because I hadn't rebuilt all the dependencies on nsIAccessible. Reversing the backout and marking FIXED again. Sorry about that.
Status: REOPENED → RESOLVED
Closed: 15 years ago15 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.