188525 - links in W3C CSS3 specs don't work

Status: RESOLVED FIXED

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

Description

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

Clicking on links in W3C specs doesn't work anymore.

Steps to reproduce:
  1. load http://www.w3.org/TR/css3-box/
  2. click on any of the links

Actual results:
  Nothing

Expected results:
  Link followed, whether anchor, separate page, or mailto.

This broke between builds 2003-01-04-21-trunk and 2003-01-05-05-trunk, which
gives a range of checkins:
http://bonsai.mozilla.org/cvsquery.cgi?treeid=default&module=all&branch=HEAD&branchtype=match&dir=&file=&filetype=match&who=&whotype=match&sortby=Date&hours=2&date=explicit&mindate=2003-01-04+21%3A00&maxdate=2003-01-05+05%3A30&cvsroot=%2Fcvsroot
so I'm giving the bug to bzbarsky (bug 126072).

I suspect the stylesheet is doing something interesting.

Comment 1

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

Attachment: simplified testcase

Comment 2

[Ruslan Ismailov]

WFM with Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.3b) Gecko/20030110
on WinXP. And I read this spec every day -- newer see any trouble. Strange.

Comment 3

[Christian :Biesinger (don't email me, ping me on IRC)]

WFM, win98, 2003010808
probably linux specific?

Comment 4

[Chi-Thanh Christopher Nguyen]

WFM Mozilla/5.0 (X11; U; SunOS sun4u; en-US; rv:1.3b) Gecko/20030110

Comment 5

[tenthumbs]

WFM with a gcc3.2.1 cvs build from this morning. There are a lot of warnings though.

Comment 6

[tenthumbs]

Sorry about the spam but I forgot to say it's a Linux build.

Comment 7

[Boris Zbarsky [:bzbarsky]]

David, I can't reproduce this with either the 2003-01-06-05 or 2003-01-10-08
linux nightly.... both the CSS3 spec and the testcase work for me....

Comment 8

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

Maybe it's the :hover rule in my user stylesheet.  I'll attach a testcase that
contains my user stylesheet as well.

Comment 9

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

Attachment: simplified testcase

This also shows the problem on a clean profile (which my previous one didn't).

Comment 10

[tenthumbs]

In the latest attachment, clicking on the link indeed fails but clicking a
second time succeeds. Odd.

Comment 11

[Boris Zbarsky [:bzbarsky]]

Hmmm, I still can't reproduce, even with the new attachment.. (though I can
imagine that a reframe there may confuse the ESM and not deliver the event
properly?)

Comment 12

[Boris Zbarsky [:bzbarsky]]

David, are you still seeing this?  I've never managed to reproduce it... 

Comment 13

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

I saw it yesterday, trying to use the css3-selectors spec.

Comment 14

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

(And yes, I still see the problem with the testcase.)

Comment 15

[Boris Zbarsky [:bzbarsky]]

OK.  I'll try to figure out how to reproduce it... (the testcase and spec both
work with a nightly build from mozilla.org for me....)

Comment 16

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

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

Comment 17

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

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

Comment 18

[Doug Daniels]

This appears in 1.3b as well.  There is a slightly simplified testcase in Bug
193034.

Comment 19

[Boris Zbarsky [:bzbarsky]]

Yeah, I can tell that people are seeing it... I _still_ can't reproduce this
with current builds or 1.3b, neither my own builds nor those from mozilla.org,
with testcases from this bug, bug 193034 or bug 193006.

So... time to start looking for far-fetched theories....  What focus setup are
people using with their windowmanagers?  Mine's sloppy-focus (focus on
mouse-enter, don't blur on mouse-exit).  What's yours David, Doug, Edwin?

Comment 20

[Doug Daniels]

I'm using Sawfish window manager version 1.2-4 from the Debian unstable
distribution.  I have it on the equivalent of strict-focus, that is I have to
click before it works.

What are you using, Boris?

Comment 21

[Boris Zbarsky [:bzbarsky]]

Like I said, "sloppy-focus" (with AfterStep).  I also just tried both that and
click-to-focus with fvwm2 and still can't get the bug to happen.  :(

Does changing your focus model to focus-follows-mouse affect this bug at all? 
If not, then I'll keep looking for other possible differences...

Comment 22

[Doug Daniels]

No, the bug still persists with sloppy focus for me.

Comment 23

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

I stepped through this in the debugger and found that things stop following the
expected codepath in nsEventStateManager::CheckForAndDispatchClick, where
|aEvent->clickCount| is 0, so no click event is fired.

Comment 24

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

And that's because in nsEventStateManager::SetClickCount,
mLastLeftMouseDownContent is a paragraph instead:

(gdb) x/wa *(void**)mLastLeftMouseDownContent
0x41b2b708 <_ZTV22nsHTMLParagraphElement+8>:    0x416de524
<_ZN22nsHTMLParagraphElement14QueryInterfaceERK4nsIDPPv>
(gdb) x/wa *(void**)mouseContent
0x41b40aa8 <_ZTV10nsTextNode+8>:        0x4199b500
<_ZN10nsTextNode14QueryInterfaceERK4nsIDPPv>

Comment 25

[Boris Zbarsky [:bzbarsky]]

jkeiser, any idea what's up?

Comment 26

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

The reason GetFrameForPoint seems to be returning the paragraph is that the
first child of the paragraph is an inline whose mContent is an
nsHTMLAnchorElement (as it should be), mPseudoTag is null (indicating no :before
content), mNextSibling is null (consistent with there being no :before content),
and has an mRect of {0,0,0,0} with the NS_FRAME_FIRST_REFLOW bit still set.

Comment 27

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

(And, FWIW, the first of the duplicates may not really be a duplicate, although
I suspect it is.)

Comment 28

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

Attachment: simpler testcase

The counter content can be replaced by having no 'content' declaration at all.

Comment 29

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

I suspect that checking in the latter part of the nsFrameManager.cpp changes
that |display->mDisplay != NS_STYLE_DISPLAY_NONE && content->ContentCount() > 0|
(the same checks nsCSSFrameConstructor::CreateGeneratedContentFrame makes) would
fix this bug by avoiding constant reframing for every style change.  Perhaps we
make a function for that check (or maybe even do it in ProbePseudoStyleContext).
 Thoughts?

Comment 30

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

Attachment: patch (diff -u)

Comment 31

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

Attachment: patch (diff -uw)

Comment 32

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

Taking.

Comment 33

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 115144 [details] [diff] [review]
patch (diff -uw)

>+    // XXXldb What is contentCount for |content: ""|?

"1" as far as I can tell by reading the code (I could check for sure with
printf, if you want).

So this fixes the testcases in this bug by preventing a reframe when going from

no before style

to 

a:before { }

Don't things still break if we go from no before style to a:before { content:
"" } or something like that?  Or are things ok then because we have an actual
content object hanging around there?

Comment 34

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 115144 [details] [diff] [review]
patch (diff -uw)

I guess my real question is, "what was broken, and why does this patch fix it?"
 ;)

I can try to figure this on my own, but I won't have much free time in the next
week or two.  :(

Comment 35

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

What was broken is that there were cases where ProbePseudoStyleContext returned
a style context but we didn't generate a pseudo-element frame.  This meant we
reframed for every style change.

Comment 36

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 115144 [details] [diff] [review]
patch (diff -uw)

Right, I got that part (and this patch is good for fixing that and should go
in).

What I'm not seeing is why this lead to clicks failing and why clicks won't
similarly fail in cases when the reframe _is_ needed (eg an :active style that
adds or removes generated content).

Comment 37

[Asa Dotzler [:asa]]

Comment on attachment 115144 [details] [diff] [review]
patch (diff -uw)

a=asa (on behalf of drivers) for checkin to 1.3

Comment 38

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

Fix checked in to trunk, 2003-02-23 09:28 PST.  Fix checked in to
MOZILLA_1_3_BRANCH, 2003-02-24 13:15 PST.

Comment 39

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

Filed bug 194776 about the general problem of processing events between the
reframe and the reflow.