Closed Bug 90664 Opened 21 years ago Closed 17 years ago

[quirks] Problem with nested links and tables

Categories

(Core :: DOM: HTML Parser, defect, P3)

Sun
Solaris
defect

Tracking

()

RESOLVED FIXED
Future

People

(Reporter: tony, Assigned: mrbkap)

References

()

Details

(Keywords: compat, platform-parity, testcase)

Attachments

(2 files)

From Bugzilla Helper:
User-Agent: Mozilla/5.0 (X11; U; SunOS sun4u; en-US; rv:0.9.1+) Gecko/20010628
BuildID:    201062822

Clicking on any of the left side links(on the www.nsxuppereast.com site) (for
example "EVENTS") results in error about url not found - seems the space in the
url is not being parsed correctly. If the EVENTS link is selected to open in a
new window, then it works ok.

Reproducible: Always
Steps to Reproduce:
1.Go to http://www.nsxuppereast.com
2.Click on "Events" (left side of page)
Works for me on today's branch build, linux
http://www.nsxuppereast.com/NewFiles/Upcoming%20Events.html %20 being the char
that handles the space
WFM: linux 2001071223
WFM 2001071214 Win2k. Reporter: Try a more recent build.

FWIW, this bug did seem to occur on builds from a couple days ago, but I don't
see it in the current builds.
Status: UNCONFIRMED → RESOLVED
Closed: 21 years ago
Resolution: --- → WORKSFORME
Technically, accroding to RFC 1738, a URL with an embedded space is an invalid 
URL.
Garth: Good point on the RFC. Either way, though, this bug is still RESOLVED.
This still fails for me (Build ID: 2001071803 Windows 2000) and (Build ID:
2001071722 Solaris)

When I click on the 'Events' link on www.nsxuppereast.com I get the following
message:

Not Found
The requested URL /NSX UpperEast.data/Components/Component.html was not found on
this server.
Status: RESOLVED → UNCONFIRMED
Resolution: WORKSFORME → ---
hard to believe that we do this right on mac, win32 and linux and not solaris.
Over to necko.
Assignee: asa → neeti
Component: Browser-General → Networking: HTTP
QA Contact: doronr → benc
Okee.  Here is the deal.  the page has:

<a href="../NSX%20UpperEast.data/Components/Component.html">
<table> <tr> <td>
<a href="NewFiles/Upcoming%20Events.html">EVENTS</a>
</table></tr></td>
</a>

Note the <a> tag around the table.  Now comes the interesting part.  On Linux,
clicking on that link goes to "NewFiles/Upcoming%20Events.html" (linux build
2001-07-26-08).  On Solaris (solaris build 2001-07-25-22) it goes to
"../NSX%20UpperEast.data/Components/Component.html", which gives a 404.

_But_ only the actual links in the table are active.  Over to parser, but it
could be that the code that's supposed to get the href of the parent anchor is
failing...
Assignee: neeti → harishd
Status: UNCONFIRMED → NEW
Component: Networking: HTTP → Parser
Ever confirmed: true
QA Contact: benc → bsharma
I can swear ( umpt.. )that this is not a parser issue! 
Okee.  :)  Over to XP Apps then.
Component: Parser → XP Apps
Keywords: pp
um. reassign for real.
Assignee: harishd → pchen
QA Contact: bsharma → sairuh
Garth: I think a space that is encoded is acceptable, unecoded spaces are what
is prohibited...

Probably the summary should be updated as well.
I can reproduce this problem on both Solaris and Linux (intel).
When I move mouse over link, status bar displays
'http://www.nsxuppereast.com/NewFiles/Upcoming%20Events.html',
but when I click the link, browser goes to
http://www.nsxuppereast.com/NSX%20UpperEast.data/Components/Component.html

Mozilla versions:
Linux:   Mozilla/5.0 (X11; U; Linux i686; en-US; rv:0.9.4+) Gecko/20010921
Solaris: Mozilla/5.0 (X11; U; SunOS sun4u; en-US; rv:0.9.4+) Gecko/20010910
Assignee: pchen → attinasi
Component: XP Apps → Layout
QA Contact: sairuh → petersen
Umm, I don't know why this is an xpapps issue. This is a layout issue. 
Personally, if you're going to do this:

<a href="foopy.html">
<table>
<a href="bugger.html"> ... </a>
</table>
</a>

ou deserve to be slapped upside the head with a cold fish of some kind. Or, 
better yet, don't use GoLive. Can't you use FrontPage like everyone else? 
(Besides, I don't see them disparaging FrontPage nor Microsoft, so they aren't 
violating the new FP 2002 EULA ;-) )

The question is why is the encompassing anchor href getting precedence over the 
actual anchor that the user is clicking. By the way, this works ok under IE, 
surprise.

Reassigning to layout
Harish, ol' buddy, I think it is a parser issue, actually.

The markup

<a href="http://www.mozilla.org/">
 <table>
  <a href="http://www.attinasi.org/"> Linq </a>
 </table>
</a>

(also in attached testcase) produces this content model:

   a@02F52470 href=http://www.mozilla.org/ refcount=4<
     Text@02F52300 refcount=3<\n>
     Text@02F53090 refcount=3<Linq \n>
     table@02F521E0 refcount=4<
       Text@02F52120 refcount=2<\n >
     >
     Text@02F52180 refcount=3<\n>
   >

Note that there is only ONE anchor - the inside anchor is not in the content
model at all, so it will be hard for layout to do anything with it ;)

Interestingly, if the markup is changed to put in a TR and TD as it should have,
then the problem goes away.
Assignee: attinasi → harishd
uh! :-/ ok ok....it's the parser

<a href="http://www.mozilla.org/">
<table>
<a href="http://www.attinasi.org/"> Linq </a>
</table>
</a>

Basically, <a> cannot nest. The inner <a> is misplaced inside the table and
hence is getting thrown out of <table>. That is, inner <a> is getting inserted
right after outer <a>. This causes the parser to discard inner <a>. 

May be we should close out the outer <a> on encountering <table>. 

Status: NEW → ASSIGNED
Changing component.
Status: ASSIGNED → NEW
Component: Layout → Parser
Keywords: qawantedtestcase
QA Contact: petersen → moied
Summary: A 'space' in URL is not being parsed correctly → [quirks]A 'space' in URL is not being parsed correctly
Priority: -- → P3
Target Milestone: --- → mozilla0.9.6
The problem ( demonstrated with attinasi's testcase ) is caused by
nsHTMLTokenizer::ScanDocStructure, that checks for wellformedness. If well
formed the parser would allow inline ( ex. <A> ) to contain block ( <TABLE> ).
There is yet another bug ( bug 100397 ) that suggests disabling
ScanDocStructure. I'm sort a convinced that this feature should be diabled. Marc?
Status: NEW → ASSIGNED
patch in bug 100397 should fix this problem too.
out of time :-( --> 0.9.7
Target Milestone: mozilla0.9.6 → mozilla0.9.7
Keywords: compat
Unfortunately, disabling the check for wellformedness seems to have a negative
performance impact. So, there is no quick fix to this problem. Moving to 0.9.9
Target Milestone: mozilla0.9.7 → mozilla0.9.9
This bug has been marked "future" because the original netscape engineer working 
on this is over-burdened. If you feel this is an error, that you or another
known resource will be working on this bug,or if it blocks your work in some way 
-- please attach your concern to the bug for reconsideration. 
Target Milestone: mozilla0.9.9 → Future
This has *nothing* to do with spaces. Changing summary.
Summary: [quirks]A 'space' in URL is not being parsed correctly → [quirks] Problem with nested links and tables
*** Bug 145961 has been marked as a duplicate of this bug. ***
Attached patch patch v1Splinter Review
This patch actually fixes a couple of bugs. The idea is to enable RS handling
in more situations, namely, those where we are going to have nested anchor
(<a>) tags. This makes us more compatible with the W3C specs and also fixes the
remaining issue in this bug.
Assignee: harishd → mrbkap
Attachment #171847 - Flags: superreview?(dmose)
Attachment #171847 - Flags: review?(jst)
Comment on attachment 171847 [details] [diff] [review]
patch v1

r=jst
Attachment #171847 - Flags: review?(jst) → review+
Comment on attachment 171847 [details] [diff] [review]
patch v1

Switching this request to brendan since I've seen him sr stuff in htmlparser
code and because dmose is swamped.
Attachment #171847 - Flags: superreview?(dmose) → superreview?(brendan)
Comment on attachment 171847 [details] [diff] [review]
patch v1

Looks righteous to me.

/be
Attachment #171847 - Flags: superreview?(brendan) → superreview+
Fix checked in.
Status: ASSIGNED → RESOLVED
Closed: 21 years ago17 years ago
Resolution: --- → FIXED
*** Bug 286242 has been marked as a duplicate of this bug. ***
Didn't attachment 171847 [details] [diff] [review] also fix http://secunia.com/advisories/14568/ ? I think
it did, but I could be (totally) wrong, so that's why I ask.
(In reply to comment #32)
> Didn't attachment 171847 [details] [diff] [review] [edit] also fix http://secunia.com/advisories/14568/
? I think
> it did, but I could be (totally) wrong, so that's why I ask.

It partially fixed it, but at a higher level than a correct fix would be
(namely, in the HTML parser rather than in event handling). See bug 288460 for
another version of the same problem.
*** Bug 139626 has been marked as a duplicate of this bug. ***
You need to log in before you can comment on or make changes to this bug.