369015 - [FIX] Hoist TryToScrollToRef to nsContentSink.

Status: RESOLVED FIXED

(Core :: XML, defect)

Description

[Boris Zbarsky [:bzbarsky]]

We should be doing this for XHTML too.

Comment 1

[Boris Zbarsky [:bzbarsky]]

Attachment: Proposed fix

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.

Comment 2

[Boris Zbarsky [:bzbarsky]]

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).

Comment 3

[Robert Longson [:longsonr]]

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

Comment 4

[Boris Zbarsky [:bzbarsky]]

> Should this be : 1?

Yes, it should.  I'll fix before landing.

Comment 5

[Jonas Sicking (:sicking) No longer reading bugmail consistently]

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...

Comment 6

[Boris Zbarsky [:bzbarsky]]

Attachment: Updated to comments

Comment 7

[Boris Zbarsky [:bzbarsky]]

Fixed