45210 - :hover and abs pos results in uneeded reflows due to stylecontext parentage being reversed

Status: VERIFIED FIXED

(Core :: CSS Parsing and Computation, defect, P1)

Description

[Matthias Kerkhoff]

From Bugzilla Helper:
User-Agent: Mozilla/4.73 [en] (WinNT; U)
BuildID:    20000071108

This report probably shows two different bugs. Not sure. Please read on. The 
attached testcase shows the definition of an incomplete CSS border shorthand 
(missing color) with an hover selector. IMHO this construct is parsed/applied in 
the wrong way.

Reproducible: Always
Steps to Reproduce:
0. Close Mozilla and restart it!
1. Load the attached testcase
(2. you may run into bug 45209, incorrect layout)
3. Move the mouse across the browser window and notice the effects.

Actual Results:  4. Suddenly, the images/the document will be reflowed. (This is 
caused from the hover selector, I assume.
5. Maybe - I'm not definetly sure - the effect also occurs if the mouse is moved 
over an unrelated element (f.e. the H1/H3 tags). I don't know enough about the 
propagation of :hover in this szenario.

							

Expected Results:  a) Initial layout as described under #45209
b1) The selector should (IMHO) select only IMG elements (if any).
b2) The imcomplete shorthand should be either ignored or parsed into a no-border 
style
b3) All elements in the parent chain of all IMGs have an initial (complete) 
no-border style, ie. border:0px solid black.
b1+b2+b3) There should not be any reflow, because changing from no-border to 
no-border should be a visual no-op.


This bug may be restricted to standard mode.		
This bug may be related to #45209 - using tables with position:absolute makes 
things definetly worse - but has also aspects which are unrelated to #45209.

Comment 1

[Matthias Kerkhoff]

Changed component - sorry.

Comment 2

[Matthias Kerkhoff]

Attachment: testcase

Comment 3

[Hixie (not reading bugmail)]

This has nothing to do with borders. I modified the testcase to not mention
borders anywhere, and to not apply the :hover to IMG elements, and the problem
is still present.

I will attach a simpler test case.

Comment 4

[Hixie (not reading bugmail)]

Attachment: Simplified testcase, self explanatory. Strange reflows occur.

Comment 5

[Hixie (not reading bugmail)]

To reproduce: Hover over the text in the latest attachement.

Results: Strange reflows in the absolutely positioned table.

Expected Results: The :hover rule should have NO effect, since it refers to
nonexistant elements.

Tested on Mozilla 2000072008 build.

Comment 6

[Hixie (not reading bugmail)]

:hover and abs pos are CSS2, not CSS1.

Comment 7

[Marc Attinasi]

I reduced the tescase further: I removed everything but the abs. pos. table and 
it flashes when I hover over the table, click on the body, or mouse-into the 
body (using Viewer). I'll see what is happening in SelectorMatches because, as 
Ian mentioned, the selector should match nothing.

Comment 8

[Marc Attinasi]

This may be one of the cases where having the style context parentage out of 
synch with the frame parentage is causing grief. 

When FrameManager::ReResolveStyleContext is called for the TableOuterFrame 
content node, it is calling aPresContext->ResolveStyleContextFor on the 
TableOuterFrame but is passing in the parent style context of the parent frame, 
in this case the Body element. However, the parent style context for the 
TableOuterFrame's context is the TableInnerFrame's context, not the Body 
element's context, so when CaptureChange is called to compare the old and new 
contexts, if returnes out REFRAME and the frametree gets rebuilt.

Given this particular code path, it appears that there are at least some serious 
performance impacts to not having the style tree match the frame tree - Could 
CaptureChange also err the other side and indicate less change than is really 
required because the context of the parent causes the child's new context to be 
closer to what it was then what it really should be? This looks like it could be 
illustrative of potential problems with other frames who's style context 
heirarcies do not match the frame tree's heirarchy (like several XBL frames). 
CC'ing Hyatt since we hav been discussing this issue in the context of another 
problem.

Excellent find, Matthias - thanks for reporting this, and thanks to whoever CC'd 
me on it.

Comment 9

[rods (gone)]

*** Bug 46099 has been marked as a duplicate of this bug. ***

Comment 10

[Hixie (not reading bugmail)]

As per meeting with ChrisD today, taking QA.

Comment 11

[Marc Attinasi]

Critical for beta3: problems with reframing when style is re-resolved due to the 
inverse relationship of the stylecontext in relation to the corresponding 
frames.

Comment 12

[Marc Attinasi]

Chris, it may be possible to get the style system to support style context 
parentage that is mismatched to the frame parentage: if style can call into a 
frame ask it for the parent-context, then it can use that parent context instead 
of the parent frame's context. If the frame chooses not to provide this 
parent-context then style will do what it does now, which is just use the parent 
frame's context. This can thus be implemented as a new method in nsIFrame and a 
default implementation can be created to return NULL, whereas the table frames 
can return the correct parent context.

Style code wil have to change in ReResolveStyle where it walks the frame tree 
resolving style changes, but I think the change will be small. We need to talk 
it over to make sure we want to do this first, and of course if changing the 
table code is easier then that would be better.

Comment 13

[leger]

How easy is it to workaround or avoid this problem?

Comment 14

[Marc Attinasi]

I don't know of any way to 'workaround' this problem, other than avoiding the 
use of tables in web sites (which is clearly not a viable option).

Comment 15

[Christine Hoffman]

Tested using the following builds:
Win: 08_30_04_m18
Mac: 08_30_08_m18
Linux: 08_30_04_m18

Using Ian's testcase from 7/21, crashed on Win and Linux after hovering. 
Talkback incident #16599830. Details:

1. Open page
2. Hover over lines/images
3. Crash with following details:
NETSCP6 caused an invalid page fault in module <unknown> at 00de:00000017. 
Registers: EAX=0fb6d3f0 CS=0177 EIP=00000017 EFLGS=00010207 EBX=00000000 SS=017f 
ESP=0068f760 

Call Stack:    (Signature = 0x00000017 92161e36) 
0x00000017 
nsView::HandleEvent 
[d:\builds\seamonkey\mozilla\view\src\nsView.cpp, line 329]
nsView::HandleEvent 
[d:\builds\seamonkey\mozilla\view\src\nsView.cpp, line 346]
nsViewManager2::DispatchEvent
[d:\builds\seamonkey\mozilla\view\src\nsViewManager2.cpp, line 1429]
HandleEvent
[d:\builds\seamonkey\mozilla\view\src\nsView.cpp, line 68]
nsWindow::DispatchEvent
[d:\builds\seamonkey\mozilla\widget\src\windows\nsWindow.cpp, line 618]
nsWindow::DispatchWindowEvent 
[d:\builds\seamonkey\mozilla\widget\src\windows\nsWindow.cpp, line 635]
nsWindow::DispatchMouseEvent 
[d:\builds\seamonkey\mozilla\widget\src\windows\nsWindow.cpp, line 3812]
ChildWindow::DispatchMouseEvent
[d:\builds\seamonkey\mozilla\widget\src\windows\nsWindow.cpp, line 4020]
nsWindow::ProcessMessage 
[d:\builds\seamonkey\mozilla\widget\src\windows\nsWindow.cpp, line 2909]
nsWindow::WindowProc 
[d:\builds\seamonkey\mozilla\widget\src\windows\nsWindow.cpp, line 884]
KERNEL32.DLL + 0x363b
(0xbff7363b) 
KERNEL32.DLL + 0x242e7
(0xbff942e7) 
0x00688b3e 

Comment 16

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

Maybe retry using an 8-31 build?  A major crash in nsView::HandleEvent was fixed
on 8-30.

Comment 17

[karnaze (gone)]

Marc, I'm giving this back to you since I think it will be easier to change the 
style code than the table code. I tried doing the parentage the other way in 
tables and ran into problems which I can't seem to recall. We need to discuss 
this further.

Comment 18

[Christine Hoffman]

Tested on Win 98 with 8/31/12 build and no crash. Will try on Linux tomorrow but 
as David indicated, the problem went away with a new build.

Comment 19

[Phil Peterson]

PDT thinks this sounds like a P3, as long as it doesn't crash (which it doesn't
today)

Comment 20

[Marc Attinasi]

Setting priority to P1 (leaving PDTP3 alone for PDT use).

There is a related problem with HTMLButtonControlFrame children having the wrong 
parent style contexts. This needs to be fixed along with this bug I think, to 
avoid the same problem there.

Comment 21

[Marc Attinasi]

Fixed. This required changes to: nsIFrame.h, nsFrame.h, nsFrame.cpp, 
nsFrameManager.cpp, nsTableFrame.h, nsTableFrame.cpp, nsTableOuterFrame.h, 
nsTableOuterFrame.cpp, and nsHTMLButtonControlFrame.cpp

Comment 22

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

Isn't similar work done to enable the .offsetParent DOM property?

Comment 23

[Marc Attinasi]

Jonas, I'm not sure what you are refering to - can you point me to something 
more specific to look at please? I'm guessing that you mean that some DOM code 
is making assumptions about the frame tree matching the style tree? Thanks.

Comment 24

[Hixie (not reading bugmail)]

VERIFIED FIXED on Windows 2000 Commercial Build 2000091308 and Linux Commercial 
Build 2000091312 using second testcase.