Last Comment Bug 1413 - [EVENTTARG] [DOGFOOD] event handling on right-floating images causes links not to work
: [EVENTTARG] [DOGFOOD] event handling on right-floating images causes links no...
Status: VERIFIED FIXED
[PDT-][M12 STAY]
:
Product: Core
Classification: Components
Component: Layout (show other bugs)
: Trunk
: x86 Windows 95
: P3 normal (vote)
: M12
Assigned To: kinmoz
: Chris Petersen
Mentors:
http://www.fas.harvard.edu/~dbaron/vi...
: 1546 (view as bug list)
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 1998-11-18 13:12 PST by David Baron :dbaron: ⌚️UTC+2 (review requests must explain patch)
Modified: 2007-12-10 14:12 PST (History)
7 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
a simple test case showing the bug (774 bytes, text/html)
1999-10-21 20:01 PDT, kipp
no flags Details

Description David Baron :dbaron: ⌚️UTC+2 (review requests must explain patch) 1998-11-18 13:12:07 PST
Clicking on a link in one of the non-floating parts of this document causes the
text to reflow over top of some of the nearby floating elements, and moves the
link so that it has not been clicked (and one has to find it in the mess).  This
is when I have the environment variable NG_REQUEST_VER=5.0 .

Also, (I think I reported this before, but I can't find it), none of the links
in the floating sections actually work.  The cursor doesn't change and clicking
doesn't do anything.
Comment 1 kipp 1998-11-23 14:25:59 PST
so I've checked in fixes so that (a) the links work in the floating sections and
(b) if you float an image and it's in an <A...> tag that the cursor will change
properly.

It's still a mystery as to why the page changes shape when a link is clicked and
it's also a mystery as to why the link isn't loading.
Comment 2 kipp 1998-11-23 19:34:59 PST
I figured out what is causing this weirdness.

Basically, when we click on a link we start a new document load. Sometime in
there we reflow the document *without the scrollbar*. The result is that the
document shifts slightly, and depending on the layout (dbaron's page is hosed
for other reasons) you get strange results.

I'm reassigning to michaelp to own this one and request that he own the issue
(even if troy ends up doing all the work :-)

michael: please reassign to me after the link clicking issue is done because
there are still some serious layout problems on this page.
Comment 3 michaelp 1998-11-23 19:52:59 PST
back to your end of the court...
Comment 4 kipp 1998-11-23 20:04:59 PST
*** Bug 1546 has been marked as a duplicate of this bug. ***
Comment 5 kipp 1998-11-23 20:06:59 PST
After furthur investigations, I've determined that it is an incremental reflow
bug caused by the event state manager triggering content-changed reflows for the
frames contained by the A tag that was clicked on.

The reflow is correct: in order to reapply style in this situation and switch to
the "active" state we need to do this; however, we need more data to get the
reflow right.

peter/tom: how do we express this properly for notification purposes so that the
layout code knows what to do? Should we just funnel this into the style-changed
system somehow?
Comment 6 joki (gone) 1998-11-24 00:00:59 PST
Yes, we should and the code that's in there now is incorrect.  I'd been meaning
to change all that but hadn't gotten around to it.  I didn't realize it was
causing a problem as is.  It also isn't working at the moment so I'm going to
clip it for now.
Comment 7 joki (gone) 1998-11-24 00:20:59 PST
I'm going to close this bug since I already have a bug for the non-active link
issue.  For the moment I've #ifdef'd the offending code since it wasn't working
anyway.  For anyone interested the non-active link bug number is 577
Comment 8 kipp 1998-11-24 09:13:59 PST
I wanted the bug left open after the event issue was resolved so I'm re-opening
it - the page has other layout issues on it, not just the event issue.
Comment 9 David Baron :dbaron: ⌚️UTC+2 (review requests must explain patch) 1998-11-24 10:01:59 PST
The floated images that are links still aren't treated as links... I thought I
heard a comment that that was fixed, but I guess not...
Comment 10 joki (gone) 1998-11-24 13:39:59 PST
Okay, there are two problems which cause the image links not to work.  One, I
messed up one of the coordinate transforms in the frame hierarchy.  This is
fixed.  Two, the P's overlap the floating images and are currently stealing the
events.  This isn't yet fixed.
Comment 11 David Baron :dbaron: ⌚️UTC+2 (review requests must explain patch) 1998-12-03 11:16:59 PST
There is now a band at the top of each of the two floats where the links work.
That is, the links work for the upper 2/3 of the first and second images, since
the second through fourth images are in the same float.
Comment 12 joki (gone) 1998-12-03 12:50:59 PST
Yes, that's the same behavior I'm seeing.  When running a debug build you can
turn on Debug | Visual Debugging and see exactly where the paragraph overlaps
the floats and steals the events.
Comment 13 David Baron :dbaron: ⌚️UTC+2 (review requests must explain patch) 1998-12-07 13:15:59 PST
I think the area where the clicking is working is in the margin between the
paragraphs.  It's not working where the float has the parapgraph underneath it.

I'm changing this bug title from "Clicking on link causes reflow over floats"
to "event handling on right-floating images causes links not to work", since
the original bug title was fixed.

I may try and make a test case to see if this specific to being an image,
floating right, or both.
Comment 14 David Baron :dbaron: ⌚️UTC+2 (review requests must explain patch) 1998-12-07 13:39:59 PST
I made testcases showing this bug at:
http://www.fas.harvard.edu/~dbaron/tests/nglayout/floatevent.html
http://www.fas.harvard.edu/~dbaron/tests/nglayout/floatevent2.html

The second of those is the better one (it is simplified), while the first one
was my scratchwork to figure out under what conditions it occurred.  The
problem is that there are 2 DIV's, and the inner one is floating.  The reason I
do that is to control the vertical alignment (otherwise the images *should* be
higher.)  If I take out the outer DIV then the event handling works.  It has
nothing to do with left or right or with images or text.
Comment 15 David Baron :dbaron: ⌚️UTC+2 (review requests must explain patch) 1998-12-08 20:19:59 PST
BTW, I just figured out that for most of the other "serious layout problems" on
this page, NGLayout is correct.  (Although you could argue that the spec is
wrong...)  There are only two problems other than the events 1) an error in the
vertical alignment of the floating DIV and 2) the headers that aren't floating
causing a stripe to their left.

The list margins and bullet issues are my mistakes.  The bullets show up in the
correct places and there should be no left margins on the non-floating list
items.  I don't see any way to handle this that would not introduce serious
spec compliance issues in other places.
Comment 16 David Baron :dbaron: ⌚️UTC+2 (review requests must explain patch) 1998-12-08 20:32:59 PST
All issues in this page other than the event issue (the subject of this bug)
and the problems with the lists that are my fault are covered in the following
bugs: 1060, 1277, 1583.  If you don't want to find a workaround for the list
problems (and I think you should not bother), then this bug should be closed,
with kipp's approval, once the event issue is finished.
Comment 17 David Baron :dbaron: ⌚️UTC+2 (review requests must explain patch) 1999-01-28 19:15:59 PST
See my comments in bug 2010 regarding the layout.  Fixing that in a good way
might fix this problem.
Comment 18 David Baron :dbaron: ⌚️UTC+2 (review requests must explain patch) 1999-01-28 19:16:59 PST
Sorry, 2007, not 2010. (1910 is also related)
Comment 19 leger 1999-02-03 08:03:59 PST
Setting all current Open Critical and Major to M3
Comment 20 David Baron :dbaron: ⌚️UTC+2 (review requests must explain patch) 1999-03-03 18:13:59 PST
This seems related to 1277.
Comment 21 Paul MacQuiddy 1999-03-05 22:38:59 PST
per leger, assigning QA contacts to all open bugs without QA contacts according
to list at http://bugzilla.mozilla.org/describecomponents.cgi?product=Browser
Comment 22 David Baron :dbaron: ⌚️UTC+2 (review requests must explain patch) 1999-03-24 12:53:59 PST
Kipp - I find no sign that this is fixed (99-03-23 build).  However, it may be
related to the stacking issues in bug 1277 and 1910 and 2007.  Reopening bug.
Comment 23 kipp 1999-03-24 20:45:59 PST
David says that nows its a stacking-of-floats issues (probably yet-another
margin/flaoter interaction)
Comment 24 David Baron :dbaron: ⌚️UTC+2 (review requests must explain patch) 1999-03-24 21:20:59 PST
If you put STYLE="background-color: black" on the P element after the three
images, you see what's going on.  The drawing/stacking/event order is (as
sec5525c(?) shows), float background, parent background, float inline
content, parent inline content (the last 2 might be switched).  The order
should be parent background/border, float (background/border, then inline
content), then parent inline content.  I describe that in more detail
(citing the spec) in one of the other 5 or so bugs on this - I could dig
up numbers if you want.
Comment 25 David Baron :dbaron: ⌚️UTC+2 (review requests must explain patch) 1999-03-26 14:32:59 PST
You've now fixed the drawing order but not the event-handling order.
Comment 26 Hixie (not reading bugmail) 1999-09-10 14:56:59 PDT
The event handling issue is covered by bug 5693. Should this be marked a dup?
Comment 27 kipp 1999-10-21 20:00:59 PDT
Testing the waters with a dogfood marker...
Comment 28 kipp 1999-10-21 20:01:59 PDT
Created attachment 2345 [details]
a simple test case showing the bug
Comment 29 leger 1999-10-26 17:08:59 PDT
Not for dogfood...marking [PDT-].  Unless his is common for everyday work.
Comment 30 David Baron :dbaron: ⌚️UTC+2 (review requests must explain patch) 1999-10-26 18:37:59 PDT
It's not incredibly common, but I imagine I could find it on some major sites if
I looked around a bit.
Comment 31 kipp 1999-11-03 14:09:59 PST
Fixed. See the updated GetFrameForPoint method in nsContainerFrame for some
commentary that hopefully explains the fix...
Comment 32 kinmoz 1999-11-05 09:56:59 PST
Reopening this bug and reassigning it to kin@netscape.com.

With kipp and chofmann's approval, I checked in a change to

    mozilla/layout/html/base/src/nsContainerFrame.cpp  revision 1.69

that backs out Kipp's fix for bug #1413 temporarily. This allows us to get
around 2 dogfood blocker bugs (#18002 and #18006) so that we can get M11 out
the door.

I will reenable his fix after M11 branches, and then get joki or hyatt to help
fix 18002 and 18006 properly.
Comment 33 kinmoz 1999-11-05 09:56:59 PST
Clearing fixed resolution.
Comment 34 kinmoz 1999-11-05 09:58:59 PST
Accepting bug.
Comment 35 kinmoz 1999-11-23 14:27:59 PST
Fix checked into tip:

    mozilla/layout/html/base/src/nsContainerFrame.cpp revision: 1.73

Modified Kipp's original fix so that nsContainerFrame::GetFrameForPointUsing()
looks through any outside children even if it finds a normal child containing
aPoint.

r=joki@netscape.com
Comment 36 Chris Petersen 1999-12-01 16:02:59 PST
With the Dec 01 build, I'm not seeing the problem described. Marking as verified
fixed.
Comment 37 Hixie (not reading bugmail) 1999-12-20 17:24:59 PST
This bug may have regressed. See bug 21678.
Comment 38 joki (gone) 2000-03-08 22:48:33 PST
Adding EVENTTARG to summary so it'll show up for event regression testing

Note You need to log in before you can comment on or make changes to this bug.