Closed
Bug 28553
Opened 25 years ago
Closed 25 years ago
Table disappears when mouse over link (Unnecessary reframing for "special" frames)
Categories
(Core :: Layout, defect, P1)
Core
Layout
Tracking
()
VERIFIED
FIXED
M15
People
(Reporter: cesarb, Assigned: buster)
References
()
Details
(4 keywords, Whiteboard: [PDT+])
Attachments
(4 files)
From Bug Helper:
User-Agent: Mozilla/5.0 [en-US] (Linux; I)
BuildID: 1999122808
When you put your mouse over the first link in the 'library' section of
www.securityfocus.com, it changes color normally; when you put your mouse over
the title of the second article, the 'library' section turns empty.
Reproducible: Always
Steps to Reproduce:
1. Open the page
2. Go to the section titled 'library'
3. Make the mouse go over the title of the second article
Actual Results: The whole 'library' section became empty.
Expected Results: Color the link (like it does in the first article's title)
I've seen this bug for some time (and some milestones).
The Build ID is bogus -- it's in fact Debian's M13-4
Comment 1•25 years ago
|
||
You are using build 1999122808. It is about 2 months old...
Please try with a new build.
PS I do not see any problems with win98 build 2000021908.
Comment 2•25 years ago
|
||
Confirmed on Linux build 2000.02.19.09. Based on ezh's comments, marking pp.
Simplified testcase attached. Problem seems to occur because of unclosed <SPAN>
tag, the original page has a </CLASS> where there should be a </SPAN>.
Reassign to rickg/HTML Elements.
Comment 3•25 years ago
|
||
Comment 4•25 years ago
|
||
Oops, forgot to reassign.
Assignee: joki → rickg
Status: UNCONFIRMED → NEW
Ever confirmed: true
Actually, the parser is working perfectly. The content model is fine too; this
appears to be a bug in either the style system or in event handling. I'll start
with style: markA, please take a look. (BTW: the testcase is pretty good).
Assignee: rickg → attinasi
Updated•25 years ago
|
Status: NEW → ASSIGNED
I actually digged into this somewhat and it seems like it's a incremetal reflow
problem, possibly in the table code (not sure). The content model is and remains
correct even after the table in the test case disappears, but the frame
hierarchy is changed.
Mousing over the link changes the state of the link to hover, this
(incorrectly?) causes an incremental reflow that reflows (part of) the document,
during this reflow mozilla loses part of the content of the table.
Actually, mozilla doesn't allways loose the content, sometimes it *adds* copies
of the content!
If you have a look at:
http://bugzilla.mozilla.org/showattachment.cgi?attach_id=5562
you'll see tha same problem, but if you have a look at:
http://bugzilla.mozilla.org/showattachment.cgi?attach_id=5564
which is the same file but with a <B> tag removed you'll see that every time the
mouse either enters or exits the link part of the content is copied!
I think the component should be "Layout" in stead of HTML element, does anyone
disagree?
Reporter | ||
Comment 8•25 years ago
|
||
May I make some comments?
First, to whoever said my build was old: I explicitly noted that the build ID
was bogus. Somehow, all builds I get as Debian packages seem to have bogus build
IDs.
Second, IMNSHO reflowing when a link state changes to hover is the correct
behavior, since the link size or even some wierder attributes for the link might
have changed (using CSS you can do pretty much anything).
Comment 9•25 years ago
|
||
Chris, The table says it wants to be 90 twips after a reflow. This looks like
the table is reflowing on the hover, and further it looks like the Table Outer
is changing the incremental into a resize... here is the dump of the reflow
after the hover where the table asks to be 90 twips:
TO::Rfl en 0166B390 rea=1 av=(8940,UC) comp=(8910,UC) count=10
T::Rfl en 0166B3E4 rea=2 av=(8940,UC) comp=(8910,UC) count=11
TRG::Rfl 0166B450 rea=2 av=(60,UC) comp=(60,UC) count=12
TR::Rfl en 0166B490 rea=2 av=(60,UC) comp=(60,UC) count=13
TR::Rfl ex 0166B490 des=(30,60)
TRG::Rfl ex 0166B450 des=(60,60)
T::Rfl ex 0166B3E4 des=(90,90)
TO::Rfl ex 0166B390 des=(90,90)
I'm sure you can fix this faster than I can.
Also, I removed the pp keyword since this is all platforms, and I changed to
Layout
Component: HTML Element → Layout
Keywords: pp
Assignee | ||
Comment 11•25 years ago
|
||
happens on top100 site hotbot, so nominating for beta1. makes the page unusable.
Severity: normal → critical
Priority: P3 → P1
Updated•25 years ago
|
Status: NEW → ASSIGNED
Comment 12•25 years ago
|
||
Pierre, mousing over the link in the attachment is causing the table cell frame
to be destroyed and then recreated. I'm not sure why a reframe reflow is needed
or even a reflow for that matter. The table code eventually needs to be changed
to handle this situation (which can still come up when the style problem is
fixed), but the style problem should be fixed first.
Assignee: karnaze → pierre
Status: ASSIGNED → NEW
Comment 13•25 years ago
|
||
per PDT, this bug will become PDT- on Friday 2/25
Whiteboard: [PDT+] → [PDT+] Will become PDT- on 2/25
Whiteboard: [PDT+] Will become PDT- on 2/25 → [PDT+] w/b minus on 02/25
Comment 14•25 years ago
|
||
*** Bug 29068 has been marked as a duplicate of this bug. ***
Comment 15•25 years ago
|
||
Moving to PDT-. Did not make the 02/25 deadline for beta1. Putting on the
relnote keyword.
Keywords: relnote
Whiteboard: [PDT+] w/b minus on 02/25 → [PDT-] w/b minus on 02/25
Comment 16•25 years ago
|
||
*** Bug 26015 has been marked as a duplicate of this bug. ***
Comment 17•25 years ago
|
||
*** Bug 26015 has been marked as a duplicate of this bug. ***
Comment 18•25 years ago
|
||
Taking over this one to help reduce Pierre's bug count.
Assignee: pierre → attinasi
Updated•25 years ago
|
Status: NEW → ASSIGNED
Comment 19•25 years ago
|
||
The SPAN does not have to be unclosed. I can recreate the problem with the
following three criterion:
1) SPAN inside of a table cell
2) SPAN contains a <P>paragraph</P>
3) SPAN contains a link <A HREF-"foo">Link</A> after the <P>
This seems to cause a problem with the PrimaryFrame of the Anchor such that it
cannot be found, causing a reframe when we really just want to see if the
content stat has changed for the anchor:hover.
Here is the new testcase (the previous one is fine too, but is missing
the </SPAN>):
<body>
<table border=1>
<tr><td>
<SPAN>
Here we open a SPAN tag and close it after the link
<P>This Paragraph is in the span, as is the following link</P>
<A HREF="http://www.yahoo.com">Mouse over this HREF tag to see entire table
disappear</A>
</SPAN>
</td></tr></table>
</body>
I'm looking at why the PrimaryFrame is not found for the ANCHOR in
nsCSSFrameConstructor::ContentStatesChanged
Comment 20•25 years ago
|
||
It looks like the primary frame is not found because of a side effect of a
change Kipp made in how he handles blocks within inlines. I'll quote his
explanation here so I don't get it wrong (I hope he don't mind):
"Rather than have the block frame be a child frame of the inline frame (which
makes layout difficult), what he does is end the inline frame, create an
anonymous block to wrap the block frame, and then recontinue the inline frame.
For example, for something like this:
<SPAN>Some text <DIV>Nested div</DIV> more text</SPAN>
We would have frames that look like this:
inline <
text-frame: Some text
>
anonymous-block <
div <
text-frame: Nested div
>
>
inline <
text-frame: more text
>
That's what I seem to remember it would look like.
The problem here is that from a content model perspective everything is a child
of the SPAN element. However, if we have to search the tree (i.e.,
the frames are not in the map) we only end up finding what is in the first
"inline" frame
We don't know to check the anonymous-block or the inline frame below it because
the frames aren't part of the same flow and the code currently
doesn't to do anything special for anonymous blocks.
Basically if we're going to continue doing what Kipp did to the frame model then
we need to make sure the code that searches for child frames knows
how to handle it." <troy chevalier>
I verified that by putting the inline frame created for the anchor into the
primary frame map, so it can be found, the bug goes away. It looks like we need
to modify the FindFrameWithContent routine in nsCSSFrameConstructor to take into
account the case where an inline frame has created and anonymous block to wrap
the block frame.
Comment 21•25 years ago
|
||
Adding Troy, Buster to CC list.
Comment 22•25 years ago
|
||
*** Bug 30676 has been marked as a duplicate of this bug. ***
Comment 23•25 years ago
|
||
As we talked about, maybe the best solution is to put the primary frame in the
map when we know we are creating an anonymous block to wrap the block frame that
is in the inline frame.
A test case will be attached showing the same root bug in and out of a table.
OUtside of a table the same problem finding the primary frame occurs, however
the display is not as bad (it just flashes due to the reframe).
Assignee: attinasi → buster
Status: ASSIGNED → NEW
Comment 24•25 years ago
|
||
Comment 25•25 years ago
|
||
No, adding the frames to the map when we're in that situation is not a very good
solution. We would have to add all child frames recursively (which could be a
lot), and we have to make sure that when incremental reflow changes happen we do
the same.
That solution would be very fragile as well as unnecessarily increasing the
size of the map
Assignee | ||
Comment 26•25 years ago
|
||
troy is right, we would have to recursively set all frames contained by the
"special" frames into the primary frame map. Just for fun, I tried the simple
approach (3 lines of code) and while if fixes simple test cases, it doesn't fix
the general problem at all.
Status: NEW → ASSIGNED
Comment 27•25 years ago
|
||
Assignee | ||
Comment 28•25 years ago
|
||
I've started a thread on n.p.m.layout. I thought it was too involved to just
dump into the bug report. The thread is titled "Possible solution for 28553,
need some feedback" Please read the posting and respond there. Thanks.
Whiteboard: [PDT-] w/b minus on 02/25 → [PDT-]
Assignee | ||
Comment 29•25 years ago
|
||
I have a candidate fix in hand for this bug. It is not risk-free, and probably
needs to go through a review/rewrite/test cycle before it's ready for prime
time. So I agree wiht the PDT- designation. Fix will be ready when the M15
tree opens for general development.
Whiteboard: [PDT-] → [PDT-] fix in hand
Target Milestone: M15
Reporter | ||
Comment 30•25 years ago
|
||
Augh, the same bug appeared in freenet.sourceforge.net (try the FAQ link in the
middle of the text). This is beginning to annoy me.
Assignee | ||
Comment 31•25 years ago
|
||
I talked to JAR and RickG separately about this today. In each conversation, we
waffled about how it would be great to get this in, since so many pages (some
top100 pages) are effected visually, and because it's a huge performance win on
pages that exhibit the pattern. Of these effected pages, we believe most will
be have the display problems (but not the performance problems) fixed by chris
karnaze's fix for 29429. However, Chris is only 50-50 on being able to get this
in by tonight, and he is on vacation through the weekend after than. If Chris
does not check in his fix, then the fix for this bug would avoid his misbehaving
code entirely, making those problem pages layout correctly, respond much
faster, and use far less memory. The problem he is working on would persist,
but it would show up on fewer pages.
So, I propose we do this:
1. Wait and see if Chris gets the fix for 29429 in. If so, this bug is PDT-.
2. If Chris can't get in the fix for 29429, then we check in my fix for this
bug, pending review and approval by troy.
Your move, PDT team.
Whiteboard: [PDT-] fix in hand → fix in hand
Assignee | ||
Comment 32•25 years ago
|
||
Comment 33•25 years ago
|
||
The fix I just checked in for bug 29429 hides the symptoms of this bug (except
for attachment #2 [details] [diff] [review]). The extra reflows are still happening, but the table code is
handling it more gracefully.
Comment 34•25 years ago
|
||
The PDT+ plus status is provisional upon 4 things:
1. that the change go in under a pref
2. that the URL's affected (to your knowledge) are listed in this bug
3. that the related bugs are cited here too
4. the usual pre-checkin rules.
Whiteboard: fix in hand → [PDT+]fix in hand
Assignee | ||
Comment 35•25 years ago
|
||
Ok:
1) The fix is ready, moved up a level as per Troy's request. Troy, please
review.
2) The fix is controlled by a pref, as per PDT's request.
3) Add dep bug 29429
4) urls are listed, there are more I'm sure be 3 should be sufficient.
FWIW, I believe this may go a long way towards fixing other bugs as well.
Depends on: 29429
Whiteboard: [PDT+]fix in hand → fix in hand
Assignee | ||
Comment 36•25 years ago
|
||
accidentally erased the [PDT+] rick had added
Whiteboard: fix in hand → [PDT+]fix in hand
Assignee | ||
Comment 37•25 years ago
|
||
Assignee | ||
Comment 38•25 years ago
|
||
Troy: I originally had this check the pref just once, but that doesn't work
because we don't have a profile in mozilla the first time this is called, so we
don't have a prefs impl to check against yet. So until the PDT team is happy
with the fix, we have to incur the cost of hitting the pref system every time we
ask for a primary frame. I suspect we can convince them to let us remove this
very soon.
Keywords: perf
Summary: Table disappears when mouse over link following unclosed <SPAN> → Table disappears when mouse over link (Unnecessary reframing for "special" frames)
Comment 39•25 years ago
|
||
The PDT people are crazy. What do they want a pref for? The fix is fine, and
it's very safe. Yes, I think you should check it in.
My only objection to the current code is that it looks like we're calling to get
the pref service each time we enter FindPrimaryFrameFor().
That will be a performance problem. We either need to eliminate the silly pref
or find a place to cache the pref service to avoid repeatedly loading it
Comment 40•25 years ago
|
||
*** Bug 31197 has been marked as a duplicate of this bug. ***
Assignee | ||
Comment 41•25 years ago
|
||
checked in fix, but won't mark this fixed because the code includes checking a
pref every time through a frequently-called routine which is very inefficient.
AFter a day or two of widespread testing, PDT team should allow me to remove the
hacky pref.
Re-nominating on the basis of getting the pref the heck out of the code.
Whiteboard: [PDT+]fix in hand → fix checked in with pref
Comment 42•25 years ago
|
||
Are you totally you are convinced that pref is no longer needed? Will the
overhead be noticible. Can you cache the pref in a static variable?
Whiteboard: fix checked in with pref → [NEED INFO]fix checked in with pref
Comment 43•25 years ago
|
||
To add my two cents.
1. the pref was never needed in the first place. This fix is very low risk
2. yes it's a big performance problem
Assignee | ||
Comment 44•25 years ago
|
||
I am convinced the pref is unnecessary. I am certainly willing to let folks
bang on it for a day, and remove the code from the trunk and branch if
necessary.
It is not trivial to cache the pref, because the code can be called before the
pref file is loaded (the dialog that asks you to pick a profile uses core
layout.) So I would need to detect when the profile has been loaded, and I don't
know how to do that. In any event, it's not worth spending time on for code
that should be removed.
Comment 45•25 years ago
|
||
my two cents, this pref is showing up in my quantify work as well for loading a
mail folder.
Comment 46•25 years ago
|
||
Putting on PDT+ radar for beta1 to .....Please remove the pref from the trunk
this weekend or Monday, we can remove from the branch on Tues.
Whiteboard: [NEED INFO]fix checked in with pref → [PDT+]fix checked in with pref
Comment 47•25 years ago
|
||
*** Bug 31435 has been marked as a duplicate of this bug. ***
Comment 48•25 years ago
|
||
To land the "pref-removal," just page-chofmann and schedule a slot.
This seems like an easy change... so it would be nice to see it land RSN.
Whiteboard: [PDT+]fix checked in with pref → [PDT+]fix checked ; Now need to remove pref control
Whiteboard: [PDT+]fix checked ; Now need to remove pref control → [PDT+] fix (w/o pref) checked into trunk. waiting time slot for netscape beta branch
Comment 49•25 years ago
|
||
*** Bug 31644 has been marked as a duplicate of this bug. ***
Assignee | ||
Comment 50•25 years ago
|
||
fixed check in to both branch and trunk
Status: ASSIGNED → RESOLVED
Closed: 25 years ago
Resolution: --- → FIXED
Whiteboard: [PDT+] fix (w/o pref) checked into trunk. waiting time slot for netscape beta branch → [PDT+]
Comment 51•25 years ago
|
||
Verified on Linux build 2000.03.14.18.
Comment 52•25 years ago
|
||
Bug 30007, bug 30676, bug 31435, bug 31644, and bug 32052 reported mouseovers
recreating table cells etc. Some of these have been marked duplicates of this
bug. All of them, except bug 30676, still show the buggy behavior on Linux
build 2000.03.15.12-nb1b. So either these were not duplicates, or this bug
should be REOPENED.
Assignee | ||
Comment 53•25 years ago
|
||
any of those other bugs that are not fixed should be reopened
Reporter | ||
Comment 54•25 years ago
|
||
Looks like some of the bugs marked as dupes of this were neglected... bug 31137
looks more like a dupe of 29429 but it's still marked as dupe of this one (and
it's still an issue). Whoever checked and reopened the (wrong) dupes only
checked the bugs referenced by zach's posting but the posting missed half of the
dupes.
Comment 55•25 years ago
|
||
Comment 56•25 years ago
|
||
*** Bug 32421 has been marked as a duplicate of this bug. ***
Comment 57•25 years ago
|
||
Fixed in March 22 nd build for Mac, Win 98, and Linux.
Status: RESOLVED → VERIFIED
Comment 58•25 years ago
|
||
Bug 29511 which appears to be a dupe of this is not fixed. I'm using 2000040815.
You need to log in
before you can comment on or make changes to this bug.
Description
•