215465 - CSS :hover rules not working when using XML/XSLT along with an alternernate CSS

Status: RESOLVED FIXED

(Core :: XSLT, defect, P2)

Description

[Peter Ryan]

User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.5a) Gecko/20030728 Mozilla Firebird/0.6.1
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.5a) Gecko/20030728 Mozilla Firebird/0.6.1

The links on the page have a :hover rule that should cause them to change colour
when the mouse rolls over them. This works perfectly *except* when using
XML/XSLT and an alternate CSS stylesheet!

If I use (X)HTML instead of XML/XSLT then it works. If I use XML/XSLT but just a
single CSS, then it works.

The example page links to variations of the same based on HTML (both 1 and 2
CSS) and XML/XSLT (both 1 and 2 CSS).

The last link on the example page has an onmouseover JavaScript that proves that
the mouseover event is being triggered.

Reproducible: Always

Steps to Reproduce:
1. Go to http://clickindustrial.com/test/public/2003-07-31/moz-xml-style/double.xml
2. Notice that the mouse pointer *does not* cause the links to change colour
3. Click on any of the top four links and see it working correctly.




I can re-create this problem on:
  Win2K Moz-Firebird 0.6
  Win2K Moz-Firebird 0.6.1
  Win2K Mozilla-Suite 1.4
  Linux Mozilla-Suite 1.3

And the problem has been confirmed by a third party using:
Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.5b) Gecko/20030729 Mozilla
Firebird/0.6.1

The problem was first posted to
http://forums.mozillazine.org/viewtopic.php?t=17981 to try to gather more feed-back!

Comment 1

[Peter Ryan]

A new example page:
http://clickindustrial.com/test/public/2003-10-16/moz-xml-style/double.xml

In this case I have two stylesheets, both with title attributes but the titles
are different. There are no stylesheets linked as "alternate" but since there
are in effect two "preferred" stylesheets, the first one is being used and the
second is effectively being treated as "alternate".

In this case, everything works fine! If I just add the word "alternate" to the
second link, then the :hover rules stop working.

So this very nearly works -- its just slightly broken!!

Comment 2

[alanjstr]

You seem to be talking about several things at once.  You might need to provide
2 testcases.  Be sure to clearly label the current behavior vs expected behavior.

If your example has changed, then the URL at the top of this box should be
updated.  

Comment 3

[Peter Ryan]

Example of broken behaviour:
http://clickindustrial.com/test/public/2003-10-18/moz-xml-style/broken.xml

Example of expected behaviour:
http://clickindustrial.com/test/public/2003-10-18/moz-xml-style/working.xml

The CSS :hover rules applied to the hyperlink in broken.xml do not work; the
link should change colour as the mouse moves over it.

The expected behaviour is that of working.xml

A more verbose explaination of the problem is here:
http://clickindustrial.com/test/public/2003-10-18/moz-xml-style/readme.txt

And you can (even) download the whole lot as a zip from here:
http://clickindustrial.com/test/public/2003-10-18/moz-xml-style/package.zip

I'm the reporter that just keeps on giving! ;)

Comment 4

[Jonathan Louie]

i'm not up on CSS but the page does have the problem you mention and behaves
differently in IE.

Comment 5

[Boris Zbarsky [:bzbarsky]]

I bet the problem is that SignalTransformEnd() does not wait for alternate
stylesheets.   Once I have a functioning build, in a few hours, I'll test this
theory.  Not sure how that messes things up, though.

Notice that not only does :hover not work, but neither does selection.  In fact,
if you use DOM inspector, all the frames on the "double.xml" testcase are
scrunched up near the top left somewhere.  So GetFrameForPoint likely gives
pretty bogus results on that page.

Comment 6

[Boris Zbarsky [:bzbarsky]]

OK, so the problem seems to be that XSLT just screws up.  If the sheet is an
alternate, then the return value is NOT NS_ERROR_HTMLPARSER_BLOCK and the sheet
is not appended via mNotifier->AddStyleSheet().  But the mNotifier has been
passed to the CSS loader, and the loader does notify it when the sheet finishes
up.  Now when StyleSheetLoaded is called, txTransformNotifier removes it from
mStylesheets and then _unconditionally_ calls SignalTransformEnd().

So what happens in this testcase is that the sequence of events is:

1)  Transform starts.
2)  Non-alternate load starts.  One sheet added to list.
3)  Alternate load starts.
4)  Transform finishes.  SignalTransformEnd() called, does nothing because there
    is a pending sheet.
5)  Non-alternate load finishes.  SignalTransformEnd() called, no pending sheets
    known, OnTransformDone is called.
6)  Alternate load finishes.  SignalTransformEnd() called again,
    OnTransformDone() called _again_.  This causes InitialReflow() to be called
    for the second time, and fireworks ensue:

###!!! ASSERTION: initial containing block already created: 'nsnull ==
mInitialContainingBlock', file
/home/bzbarsky/mozilla/xlib/mozilla/layout/html/style/src/nsCSSFrameConstructor.cpp,
line 8795
###!!! ASSERTION: unexpected second call to SetInitialChildList: 'Not Reached',
file
/home/bzbarsky/mozilla/xlib/mozilla/layout/html/base/src/nsContainerFrame.cpp,
line 109
###!!! ASSERTION: node in map twice: 'Not Reached', file
/home/bzbarsky/mozilla/xlib/mozilla/layout/html/base/src/nsFrameManager.cpp,
line 2388

The simple fix is to change:

txTransformNotifier::StyleSheetLoaded(nsICSSStyleSheet* aSheet, PRBool aNotify)
{
    // aSheet might not be in our list if the load was done synchronously
    mStylesheets.RemoveObject(aSheet);
    SignalTransformEnd();
    return NS_OK;
}


to

txTransformNotifier::StyleSheetLoaded(nsICSSStyleSheet* aSheet, PRBool aNotify)
{
    // aSheet might not be in our list if the load was done synchronously
    if (mStylesheets.RemoveObject(aSheet)) {
        SignalTransformEnd();
    }
    return NS_OK;
}

All of this setup will need slight tweaking in bug 84582, I guess...

Anyway, over to XSLT.  You guys really don't want to be calling OnTransformDone
twice, do you?

Comment 7

[Peter Van der Beken [:peterv]]

Ugh, no. Thanks for looking bz!

Comment 8

[Peter Van der Beken [:peterv]]

Attachment: v1

Comment 9

[Peter Van der Beken [:peterv]]

Comment on attachment 134195 [details] [diff] [review]
v1

This is just bz's patch so I can sr :-).

Comment 10

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

Comment on attachment 134195 [details] [diff] [review]
v1

I think i would prefer a flag indicating that the OnTransformDone() was already
called (probably by changing mInTransform to be an enum). That seems more
robust.

Though i'm ok with this too, but please add a comment indicating why this is
needed.

r=sicking

Comment 11

[Peter Van der Beken [:peterv]]

Checked in with a comment referring to this bug.