Closed Bug 501656 Opened 11 years ago Closed 11 years ago

selected state is not exposed on selected HTML table cell

Categories

(Core :: Disability Access APIs, defect)

defect
Not set
normal

Tracking

()

RESOLVED FIXED

People

(Reporter: surkov, Assigned: surkov)

References

(Blocks 1 open bug)

Details

(Keywords: access)

Attachments

(1 file)

selected state is not exposed on selected HTML table cell
Attached patch patchSplinter Review
Assignee: nobody → surkov.alexander
Status: NEW → ASSIGNED
Attachment #386660 - Flags: review?(marco.zehe)
Attachment #386660 - Flags: review?(bolterbugz)
Comment on attachment 386660 [details] [diff] [review]
patch

r=me
Attachment #386660 - Flags: review?(marco.zehe) → review+
Comment on attachment 386660 [details] [diff] [review]
patch

1 nit:

>+  nsCOMPtr<nsIContent> content = do_QueryInterface(mDOMNode);
>+  nsCOMPtr<nsIPresShell> presShell = do_QueryReferent(mWeakShell);
>+  nsIFrame *frame = presShell->GetPrimaryFrameFor(content);
>+  NS_ASSERTION(frame, "No frame for valid cell accessible!");

We should probably avoid using NS_ASSERTION in new code, because there is so much console output already it is almost useless. Do you think NS_ABORT_IF_FALSE is maybe too strong here?

Either way r=me (the rest looks good).
Attachment #386660 - Flags: review?(bolterbugz) → review+
(In reply to comment #3)
> (From update of attachment 386660 [details] [diff] [review])
> 1 nit:
> 
> >+  nsCOMPtr<nsIContent> content = do_QueryInterface(mDOMNode);
> >+  nsCOMPtr<nsIPresShell> presShell = do_QueryReferent(mWeakShell);
> >+  nsIFrame *frame = presShell->GetPrimaryFrameFor(content);
> >+  NS_ASSERTION(frame, "No frame for valid cell accessible!");
> 
> We should probably avoid using NS_ASSERTION in new code, because there is so
> much console output already it is almost useless. Do you think
> NS_ABORT_IF_FALSE is maybe too strong here?

It might be. Though I use assertion here because I'm sure it shouldn't be happen. Nevertheless I would avoid to use ns_abort :). The fact we have heavy output in console isn't related with assertion usage :) But true we should fix these things.
checked in on 1.9.2, http://hg.mozilla.org/mozilla-central/rev/7db7d2d7dbac
Status: ASSIGNED → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.