Closed
Bug 51113
Opened 24 years ago
Closed 24 years ago
[FIX IN HAND,R=,A=] Wrong cursor on link mouseover because css cursor property is not inheriting
Categories
(Core :: DOM: HTML Parser, defect, P3)
Core
DOM: HTML Parser
Tracking
()
VERIFIED
FIXED
M18
People
(Reporter: mlei2, Assigned: attinasi)
References
()
Details
(Keywords: css2, Whiteboard: [nsbeta3-][fix in hand][PDTP3] [rtm++])
Attachments
(2 files)
|
(deleted),
text/html
|
Details | |
|
(deleted),
patch
|
Details | Diff | Splinter Review |
Look at the code for this page: http://www.tjhsst.edu/~jdanaher
This is an excerpt from the code.
<td align=center width=24% valign=top>
<a href=blue/index.html><h2 align=center>Blue</h2></a>
My standard-issue personal homepage, with a random assortment of bits
and pieces. The original resident of this site.
</td>
If you just look at this code, you'll see that the link is supposed to
encompass the word "Blue." But, if you go to this page, you will see that the
link wrongly encompasses the words "Blue" thru "resident of this site."
I'm not sure what's wrong here. It seems to have something to do with the fact
that the entire thing is inside a <td></td>. I'll attach a test case to
demonstrate what I mean.
Running build 2000083111 Windows or 2000090108 Linux.
|
||
Comment 2•24 years ago
|
||
You can't wrap a line-level element <a> around a block-level element <h2>. See
http://www.w3.org/TR/html4/struct/global.html#h-7.5.3 for the rules. Since the
specification is not absolutely clear on this, I will leave it for someone else
to mark as invalid.
I don't think that the question is one of whether the syntax is exactly
correct. Every browser must have some level of error tolerance-- in fact,
having a high level of error tolerance is desirable because humans are not
perfect and the browser should try to display "what I mean, not what I say
(write,code)."
The problem here is that in one case it renders as expected (when outside of a
<td></td>) but in another case it does not (when inside a <td></td>). If one
looked at the code, you would expect for the link to end at "Blue" even if the
syntax was not exactly according to spec.
|
|
||
Comment 4•24 years ago
|
||
I can't make the decision, only the correct module owner can, but fault
tolerance in code is not something I believe in. I believe people do make
errors, but they can correct them. It is possible for the author to know what
they meant, but in many cases it is not possible for the browser to know what
they meant when they are doing illegal stuff.
|
||
Comment 5•24 years ago
|
||
mlei: No, error tolerance is a bad thing. It is what led us into the whole
mess that the web is in now. Newer specifications such as CSS and XML follow
this view and actually say exactly how to deal with invalid things.
As you say -- every browser must have some level of error tolerance. Ours is
very low, as low as we can possibly make it without breaking the majority of
existing pages.
Harish, Rick: It seems that the content sink is using different rules for
when we are inside a TD and when we are not -- inside a TD, the </a> is being
ignored or something. Could you check it out quickly? If it is easy to fix then
we might as well fix it. If not, then let's mark it WONTFIX or INVALID.
Status: UNCONFIRMED → NEW
Ever confirmed: true
requesting beta3 status because 1) it's dataloss; and 2) it's fixed in my tree.
Oops, wrong issue. I'd still like nsbeta3+ because we improperly form the
document (and it's the most common case) and it's fixed in my tree.
Keywords: dataloss
Fixed last week.
Status: ASSIGNED → RESOLVED
Closed: 24 years ago
Resolution: --- → FIXED
New bug:
Go to the test case (attachment 13902 [details]) or the URL above
(http://www.tjhsst.edu/~jdanaher). If you move your mouse over the link, the
cursor doesn't change to the hand but instead remains an I-text cursor. This is
most likely related to the original bug that was fixed.
Build Windows 2000091008, Linux 2000090908.
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Summary: link not ending in the right place → Wrong cursor on link mouseover
|
||
Comment 10•24 years ago
|
||
Note: the original problem is fixed. The new problem concerns link handling
ONLY. Here's a min test case:
<html><body>
<a href="foo"><h2>my heading</h2></a>
</body></html>
|
|
||
Comment 11•24 years ago
|
||
That is a violation of HTML4. Does Mozilla want to deal with invalid HTML?
Inline elements must be contained within a block.
|
|
||
Comment 12•24 years ago
|
||
I'm really, really happy Joki is back to diagnose this. :)
Assignee: rickg → joki
Status: REOPENED → NEW
|
||
Comment 13•24 years ago
|
||
Okay, so I traced this through and we are walking through the correct frame
which is associated with the anchor. However, this frame, which is now a block
frame as oppposed to an inline frame, isn't picking up the style rule for anchor
as currently defined in html.css. We probably need a new style rule to deal
with block anchors. Sending back to rickg.
Assignee: joki → rickg
|
|
||
Comment 14•24 years ago
|
||
This is a MAJOR issue. It happens *all* the time on pages, and is necessary for
backward compatibility. The only remaining step is to improve our .css files.
Assignee: rickg → attinasi
|
Assignee | |
Comment 15•24 years ago
|
||
Hmmm. The problem is that the cursor property is not inheriting, and according
to the CSS2 spec it should. This can be fixed in code by altering the method
StyleColorImpl::ResetFrom to inherit the cursor, or if we are too squeemish to
change c++ code at this point some creative rules can do it too.
In html.css:
:link * { cursor: inherit; }
... Anything under a link will inherit the cursor.
I think the best way is to fix the cursor inheritance code, that way other
potential cursor bugs will be fixed as well, and we will be more adherant to the
CSS2 spec. I'll attach a patch with the change to inherit the cursor, which
fixes this bug (and potentially others).
Also, updating the summary to reflect the remaining problem.
Status: NEW → ASSIGNED
Summary: Wrong cursor on link mouseover → Wrong cursor on link mouseover because css cursor property is not inheriting
Target Milestone: --- → M18
|
|
Assignee | |
Comment 16•24 years ago
|
||
Oops, I have other changes in that file so instead of a patch I'll just post the
new version of the changed method:
void StyleColorImpl::ResetFrom(const nsStyleColor* aParent, nsIPresContext*
aPresContext)
{
if (nsnull != aParent) {
mColor = aParent->mColor;
mOpacity = aParent->mOpacity;
mCursor = aParent->mCursor; /* inherit the cursor from parent */
}
else {
if (nsnull != aPresContext) {
aPresContext->GetDefaultColor(&mColor);
}
else {
mColor = NS_RGB(0x00, 0x00, 0x00);
}
mOpacity = 1.0f;
mCursor = NS_STYLE_CURSOR_AUTO;
}
mBackgroundFlags = NS_STYLE_BG_COLOR_TRANSPARENT | NS_STYLE_BG_IMAGE_NONE;
if (nsnull != aPresContext) {
aPresContext->GetDefaultBackgroundColor(&mBackgroundColor);
aPresContext->GetDefaultBackgroundImageAttachment(&mBackgroundAttachment);
aPresContext->GetDefaultBackgroundImageRepeat(&mBackgroundRepeat);
aPresContext->GetDefaultBackgroundImageOffset(&mBackgroundXPosition,
&mBackgroundYPosition);
aPresContext->GetDefaultBackgroundImage(mBackgroundImage);
}
else {
mBackgroundColor = NS_RGB(192,192,192);
mBackgroundAttachment = NS_STYLE_BG_ATTACHMENT_SCROLL;
mBackgroundRepeat = NS_STYLE_BG_REPEAT_XY;
mBackgroundXPosition = 0;
mBackgroundYPosition = 0;
}
}
|
|
Assignee | |
Comment 17•24 years ago
|
||
|
|
Assignee | |
Comment 18•24 years ago
|
||
I attached the patch after all to make testing and reviewing easier, and because
I had to get the other changes out anyway if I plan to commit this.
General Idea: get the cursor from the parent if there is a parent, otherwise set
it to the defaul value of auto.
|
|
Assignee | |
Comment 19•24 years ago
|
||
Bumping up to P2 based upon Rick G.'s comments.
|
||
Comment 20•24 years ago
|
||
The patch is fine but I believe that it only fixes part of the problem. It seems
to me that nsTextFrame::GetCursor() should iterate through all the parents (not
just the first one) to find the first one which doesn't have NS_STYLE_CURSOR_AUTO
(and if we don't find any, set NS_STYLE_CURSOR_TEXT), a mechanism a little bit
similar to what we have for 'cursor-select' in nsFrame::IsSelectable().
When it's done, could you make sure that bug 30971 (cursor: auto does not work)
and bug 20080 (cursor: inheritance bug: not inherited inside floated span) are
still valid? Thanks.
|
||
Comment 21•24 years ago
|
||
PDT thinks this is an edge case, ->P3.
Priority: P2 → P3
Whiteboard: [nsbeta3+][fix in hand] → [nsbeta3+][fix in hand][PDTP3]
|
||
Comment 22•24 years ago
|
||
per PDT: P3-P5 priority bugs changed from nsbeta3+ to nsbeta3- since we have
more important work to do for Seamonkey. If you disagree, please state your case
in the bug report and nominate for rtm. Thanks.
Whiteboard: [nsbeta3+][fix in hand][PDTP3] → [nsbeta3-][fix in hand][PDTP3]
|
|
Assignee | |
Comment 23•24 years ago
|
||
Agreed that beta3 can live without this. However, inheriting the cursor property
should get in for RTM: it is very low risk, and it is correct behavior.
The additional problem that Pierre brought up (walking ancestors to find
non-auto cursor for text frames) can be treated as a separate bug if needed.
Keywords: rtm
|
|
Assignee | |
Comment 24•24 years ago
|
||
*** Bug 20080 has been marked as a duplicate of this bug. ***
|
||
Comment 25•24 years ago
|
||
Adding rtm+ due to low risk.
Whiteboard: [nsbeta3-][fix in hand][PDTP3] → [nsbeta3-][fix in hand][PDTP3] [rtm+]
|
|
Assignee | |
Updated•24 years ago
|
Keywords: patch
Summary: Wrong cursor on link mouseover because css cursor property is not inheriting → [FIX IN HAND] Wrong cursor on link mouseover because css cursor property is not inheriting
|
||
Comment 26•24 years ago
|
||
PDT marking [rtm need info] since the patch doesn't have code review yet.
Whiteboard: [nsbeta3-][fix in hand][PDTP3] [rtm+] → [nsbeta3-][fix in hand][PDTP3] [rtm need info]
|
|
Assignee | |
Comment 27•24 years ago
|
||
Pierre said the patch was fine, though he did bring up a related problem. See
his comment from 2000-09-15 13:52 and my response vis a vis opening another bug
for the related issue from 2000-09-26 14:07.
I'm requesting super-review for this now.
|
|
Assignee | |
Comment 28•24 years ago
|
||
I believe that the auto-cursor handling that Pierre mentioned in his comment
from 2000-09-15 13:52 is covered by bug 30971. I'm copying his comment to that
bug so we don't lose the information.
|
|
||
Comment 29•24 years ago
|
||
r=pierre. This fix is sound and safe. The rest will be fixed later. Marc moved my
comments to bug 30971.
|
||
Comment 30•24 years ago
|
||
add comments to the fix citing the bug number, and you get a=buster
|
|
Assignee | |
Comment 31•24 years ago
|
||
Have r=a and a=, remarking rtm+ and waiting for PDT approval
Summary: [FIX IN HAND] Wrong cursor on link mouseover because css cursor property is not inheriting → [FIX IN HAND,R=,A=] Wrong cursor on link mouseover because css cursor property is not inheriting
Whiteboard: [nsbeta3-][fix in hand][PDTP3] [rtm need info] → [nsbeta3-][fix in hand][PDTP3] [rtm+]
|
|
||
Comment 32•24 years ago
|
||
PDT marking [rtm++]
Whiteboard: [nsbeta3-][fix in hand][PDTP3] [rtm+] → [nsbeta3-][fix in hand][PDTP3] [rtm++]
|
|
Assignee | |
Comment 33•24 years ago
|
||
Fix checked into branch and trunk (nsStyleContext.cpp)
Status: ASSIGNED → RESOLVED
Closed: 24 years ago → 24 years ago
Resolution: --- → FIXED
|
||
Comment 34•24 years ago
|
||
verified:
2000-10-19-08-Mtrunk : win32
2000-10-19-09-MN6 : win32
2000-10-19-09-MN6 : linux
2000-10-19-08-MN6 : mac
Status: RESOLVED → VERIFIED
|
|
||
Comment 35•23 years ago
|
||
Mass removing self from CC list.
|
|
||
Comment 36•23 years ago
|
||
Now I feel sumb because I have to add back. Sorry for the spam.
You need to log in
before you can comment on or make changes to this bug.
Description
•