Closed Bug 369015 Opened 15 years ago Closed 15 years ago

[FIX] Hoist TryToScrollToRef to nsContentSink.

Categories

(Core :: XML, defect)

x86
Linux
defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla1.9alpha1

People

(Reporter: bzbarsky, Assigned: bzbarsky)

References

Details

Attachments

(1 file, 1 obsolete file)

We should be doing this for XHTML too.
Attached patch Proposed fix (obsolete) — Splinter Review
This also backs out the "fix" for bug 137632 and bug 136788, thus refixing bug 59774 for XML.

I tested, and with this patch the issues in bug 137632 and bug 136788 don't appear.
Attachment #253711 - Flags: superreview?(jonas)
Attachment #253711 - Flags: review?(jonas)
We want tests for this, but I'm not sure how to write them (want to programmatically scroll, and get current scroll position; going back/forward in history is easy).
Flags: in-testsuite?
Comment on attachment 253711 [details] [diff] [review]
Proposed fix

>Index: content/base/src/nsContentSink.h
>-  PRUint8 unused : 2;  // bits available if someone needs one
>+  PRUint8 mChangeScrollPosWhenScrollingToRef;

Should this be : 1?

>+  PRUint8 unused : 1;  // bits available if someone needs one
> Should this be : 1?

Yes, it should.  I'll fix before landing.
Comment on attachment 253711 [details] [diff] [review]
Proposed fix

>Index: content/base/src/nsContentSink.h
>@@ -252,17 +250,18 @@ protected:
> 
>   PRPackedBool mLayoutStarted;
>   PRUint8 mScrolledToRefAlready : 1;
>   PRUint8 mCanInterruptParser : 1;
>   PRUint8 mDynamicLowerValue : 1;
>   PRUint8 mParsing : 1;
>   PRUint8 mDroppedTimer : 1;
>   PRUint8 mInTitle : 1;
>-  PRUint8 unused : 2;  // bits available if someone needs one
>+  PRUint8 mChangeScrollPosWhenScrollingToRef;
>+  PRUint8 unused : 1;  // bits available if someone needs one

You should add ": 1". I'd also remove the 'unused' member since it only increases the patchsize. And it's not like we're not gonna allow using additional bits.

Though IMHO it's a bit of overkill to use bitfields here at all. It's not like we care what the size of sink objects are...
Attachment #253711 - Flags: superreview?(jonas)
Attachment #253711 - Flags: superreview+
Attachment #253711 - Flags: review?(jonas)
Attachment #253711 - Flags: review+
Attachment #253711 - Attachment is obsolete: true
Fixed
Status: NEW → RESOLVED
Closed: 15 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.