Target (Named) Anchors with Spaces are broken, HREF doesn't navigate to target

VERIFIED FIXED

Status

()

Core
Layout
P3
normal
VERIFIED FIXED
17 years ago
16 years ago

People

(Reporter: David Ciemiewicz, Assigned: jst)

Tracking

Trunk
x86
Windows NT
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [nsbeta3+] Have fix, URL)

Attachments

(1 attachment)

(Reporter)

Description

17 years ago
Clicking on an HREF reference to a named anchor with spaces should scroll the
page to bring the table element into view.

This is broken in M16, which I downloaded on July 22, 2000 for Windows NT.
This works in Netscape Communicator 4.61

<a href="#multi part name with spaces">Click Here!</a>
. . .
<table>
<tr>
<td><a name="multi part name with spaces"></a>
<p>some text
</td>
</tr>
</table>
(Reporter)

Comment 1

17 years ago
Created attachment 11768 [details]
Test case with targets inside and outside table
(Reporter)

Updated

17 years ago
Summary: Target (Named) Anchor with Spaces are broken, HREF doesn't navigate to target → Target (Named) Anchors with Spaces are broken, HREF doesn't navigate to target

Comment 2

17 years ago
I see this with Linux 2000072113 so I'm confirming.  However, I don't know if
this is a real bug or not.  Section 6.2 of the HTML 4.01 spec does include a
space in the list of valid characters for the "NAME" attribute.  Section 2.4.3
of rfc2396 specifically disallows spaces in URIs.

For your reference:
http://www.ietf.org/rfc/rfc2396.txt
http://www.w3.org/TR/html4/types.html#type-cdata
Status: UNCONFIRMED → NEW
Ever confirmed: true

Comment 3

17 years ago
Bad typo sorry, that should say section 6.2 does not include a space in the list
of valid...
Duplicate of bug 11453 ?
(Reporter)

Comment 5

17 years ago
This bug may be related to the map bug referenced.

The discussion of CDATA at http://www.w3.org/TR/html4/types.html#type-cdata is 
ambiguous. It is implied that <space> is valid character since it is "part of 
the document character set" and "user agents may leading and trailing 
whitespace" and "replace each carriage return and linefeed with a single space".

However <space> is specifically left out of the NAME definition and is 
specifically left out from the URI and # fragment discussion.

BUT <space> may be encoded as %20 and based on the discussion in 6.2,
"replace character entities with characters" (i.e. %20 with <space>).

To be pedantic, no, there should be no unescaped spaces.  HOWEVER, legacy
improper use, place the fact that every other browser on the planet does
the right thing, plus the fact that it is relatively simple to have the
user agent automagically escape spaces to %20 when referencing external
documents and internal document fragments, the best thing would be to
support spaces in this instance because of the number of websites that
will break, otherwise.  I know it ain't "right" especially in the context
of URI definitions and XML-HTML 1.0 but well, so what.

Comment 6

17 years ago
Triaging Clayton's bug list:
----------------------------

Looks very much like 11453.  Leaving the decision to Vidur. ;)
Assignee: clayton → vidur
(Assignee)

Comment 7

17 years ago
Taking this off Vidurs list, I have a fix for this in my tree, will attach
patch. (I think this is a dup but I can't find the other bug!)
Assignee: vidur → jst
(Assignee)

Comment 8

17 years ago
Accepting bug and nominating for beta3, I've seen other sites break because of
this too. Nominating for beta3 based on correctness and backwards compatibility
with 4.x, the fix is really low risk, here's the patch:

Index: base/nsDocShell.cpp
===================================================================
RCS file: /cvsroot/mozilla/docshell/base/nsDocShell.cpp,v
retrieving revision 1.173
diff -u -r1.173 nsDocShell.cpp
--- nsDocShell.cpp      2000/07/21 23:44:38     1.173
+++ nsDocShell.cpp      2000/07/26 02:04:24
@@ -3103,6 +3103,12 @@

     if (!sNewRef.IsEmpty())
     {
+        char *tmp = sNewRef.ToNewCString();
+
+        sNewRef.Assign(NS_ConvertUTF8toUCS2(nsUnescape(tmp)));
+
+        nsMemory::Free(tmp);
+
         nsCOMPtr<nsIPresShell> shell = nsnull;
         rv = GetPresShell(getter_AddRefs(shell));
         if (NS_SUCCEEDED(rv) && shell)
Status: NEW → ASSIGNED
Keywords: correctness, nsbeta3

Comment 9

17 years ago
Marking nsbeta3+...
Whiteboard: [nsbeta3+]

Updated

17 years ago
Whiteboard: [nsbeta3+] → [nsbeta3+] Have fix
(Assignee)

Comment 10

17 years ago
Fix checked in, marking FIXED.
Status: ASSIGNED → RESOLVED
Last Resolved: 17 years ago
Resolution: --- → FIXED

Comment 11

17 years ago
Verified fixed in the Aug 24th build.
Status: RESOLVED → VERIFIED
SPAM. HTML Element component deprecated, changing component to Layout. See bug
88132 for details.
Component: HTML Element → Layout
You need to log in before you can comment on or make changes to this bug.