170644 - [RR]dynamic offset changes not working on fixed-positioned elements (auto only?)

Status: RESOLVED FIXED

(Core :: Layout: Positioned, defect, P2)

Description

[Steve Catmull]

User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.1) Gecko/20020826
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.1) Gecko/20020826

If you goto http://www.w3.org/Style/CSS/ and switch the style sheet via View 
| Use Style to the "Gold (left,fixed)" stylesheet it appears correctly. 
However, if you switch from that stylesheet to the "Gold (right,fixed)", the
text makes the switch, but the left DIV remains in the left position.

If you follow the progression of "Gold (left,fixed)", "Gold (left, fixed) +
navbar", and finally "Gold (right, fixed)", the elements appears to be rendered
correctly.  

Duplicated on two different machines on two different networks (home and work)

Reproducible: Always

Steps to Reproduce:
1.  Go to http://www.w3.org/Style/CSS/
2.  Switch stylesheet using View : Use Style to "Gold (left, fixed)"
3.  Switch stylesheet using View : Use Style to "Gold (right, fixed)"
4.  NOTE:  LEFT DIV does not move to the right side.
5.  Now switch stylesheet to "Steely"
6.  Switch to "Gold (right, fixed).  Pages is rendered as intended.

Actual Results:  
Demonstrated the odd behavior (fixed layer not moving correctly) and in steps 5
and 6, how it should behave.

Expected Results:  
When "Gold (right, fixed)" stylesheet is selected, the fixed layer should have
moved to the right side.

Comment 1

[Madhur Bhatia]

Attachment: default style sheet

Comment 2

[Madhur Bhatia]

Attachment: alternate style - left , fixed

Comment 3

[Madhur Bhatia]

Attachment: alternate style -- right , fixed

Comment 4

[Madhur Bhatia]

Attachment: reduced testcase

Comment 5

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

This is an incremental reflow problem -- resizing the window makes the layout
correct itself.

Comment 6

[Madhur Bhatia]

I did not find a dup to this bug.
Confirming the bug and adding keyword 'testcase'
setting QA priority to [QA-P3] based on severity * visibility

Visibiltiy is high since it is seen in a top site http://www.w3.org/Style/CSS/
Severity is low since it is a cosmetic related issue

Comment 7

[Boris Zbarsky [:bzbarsky]]

Attachment: simpler "left" sheet

Comment 8

[Boris Zbarsky [:bzbarsky]]

Attachment: simpler "right" sheet

Comment 9

[Boris Zbarsky [:bzbarsky]]

Attachment: simpler testcase

Comment 10

[Boris Zbarsky [:bzbarsky]]

So here's what's going on:

1)  The sheet change happens
2)  We reresolve style on the tree starting at the root
3)  We do not reresolve style on frames in the fixed list on the vieport,
    because they are out of flow
4)  We get a "reflow" style hint for the body (because the margins changed)
5)  We get to the placeholders and reresolve them and their out-of-flows (the
    fixed frames), but do _not_ append them to the change list, since their hint
    is "reflow" and we already have a "parent" with a "reflow" hint in the list.
6)  We reflow.  Unfortunately, reflowing the body does not reflow the fixed-pos
    frames, since reflow does not propagate through placeholders.

So possible thoughts:

a)  Append out-of-flows to the changelist even if their hint is already present
    in the list
b)  Have reflow propagate through placeholders (we may need this anyway; eg
    consider a fixed-pos top:auto/bottom:auto frame whose placeholder moves
    because the body reflows)

Comment 11

[Boris Zbarsky [:bzbarsky]]

Yep.  As expected, adding a ReflowDirtyChild() call in
nsPlaceHolderFrame::Reflow() to schedule its out-of-flow to be reflown fixes
this bug...

That approach seems very wrong, though.  If nothing else, that should only apply
to abs/fixed pos frames.

Comment 12

[Ali Ebrahim]

Additional testcase:

http://weblogs.mozillazine.org/asa/

Steps:
1. Switch to Clean theme.
2. Switch back to DefaultStyle theme.

The text will be misaligned. Resize or refresh window and it looks fine again.

Comment 13

[Jesse Ruderman]

The "simpler testcase" attachment WFM.

Comment 14

[Boris Zbarsky [:bzbarsky]]

So does Madhur's testcase... the problem stands, however (see dependencies).  If
someone really cares, I can look up when the testcases in this bug started
working... the code changes described in comment 10 haven't happened or anything.

Comment 15

[Boris Zbarsky [:bzbarsky]]

Attachment: Possible fix

I can't reproduce with the testcases in this bug, but the patch _does_ fix the
testcases in bug 170916, bug 180242, bug 225342, bug 231962.

It does not fix the testcase in bug 180142, though I'm not sure why.

The remaining dependencies of this bug do not have usable testcases.

Comment 16

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 146862 [details] [diff] [review]
Possible fix

David, what do you think?  We need to either do this or to always reflow all
out-of-flow descendants on any style-change reflow.  I think doing this is
cheaper...

The reflow hint is the only one we need to worry about because
ApplyRenderingChangeToTree does walk all out of flows and reframing naturally
deals with out of flows.

Comment 17

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Comment on attachment 146862 [details] [diff] [review]
Possible fix

If you're confident that we won't do a reflow on something that we've already
reframed, r+sr=dbaron.	(rbs's code handles that, right?)  I can't think of any
other assumptions that might break, but there could be some...

Comment 18

[Boris Zbarsky [:bzbarsky]]

Yeah, rbs' code should handle that....

Comment 19

[Boris Zbarsky [:bzbarsky]]

Checked in.