selected state is not exposed on selected HTML table cell

RESOLVED FIXED

Status

()

defect
RESOLVED FIXED
10 years ago
10 years ago

People

(Reporter: surkov, Assigned: surkov)

Tracking

(Blocks 1 bug, {access})

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment)

Assignee

Description

10 years ago
selected state is not exposed on selected HTML table cell
Assignee

Comment 1

10 years ago
Posted 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

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+
Assignee

Comment 4

10 years ago
(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.
Assignee

Comment 5

10 years ago
checked in on 1.9.2, http://hg.mozilla.org/mozilla-central/rev/7db7d2d7dbac
Status: ASSIGNED → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.