Closed Bug 185169 Opened 22 years ago Closed 21 years ago

server-side imagemap replaces URL query rather than appending

Categories

(Core :: Layout, defect, P4)

Product:
Core
Component:
Layout
Type:
defect

Tracking

()

Status:
RESOLVED FIXED
Milestone:
Future

People

(Reporter: dave, Assigned: john)

References

(
URL
)

Details

(Keywords: regression, Whiteboard: fixed1.3.1)

Attachments

(1 file)

Patch
1.19 KB, patch
bzbarsky
: review+
bzbarsky
: superreview+
dbaron
: approval1.3-
Details | Diff | Splinter Review 
User-Agent:       Mozilla/5.0 (Windows; U; WinNT4.0; en-US; rv:1.2b) Gecko/20021029 Phoenix/0.4
Build Identifier: Mozilla/5.0 (Windows; U; WinNT4.0; en-US; rv:1.3a) Gecko/20021207 Phoenix/0.5

This worked in Phoenix 0.4; but doesn't in 0.5:

<a HREF="AddNote.php?coord=">
<img SRC="board.gif" ISMAP BORDER=0 ALT="Click to add note!">
</a>

In 0.4, the url sent is AddNote.php?coord=?x,y
In 0.5, the url sent is AddNote.php?x,y

where x,y are the coords.

(ie 0.5 has ignored the ?... appended to the href, which I don't think is right).

Reproducible: Always

Steps to Reproduce:
See above.
Also happens in Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.3a)
Gecko/20021214
Moving to Browser, setting OS to All & reassigning.
Assignee: blaker → asa
Component: General → Browser-General
OS: Windows NT → All
Product: Phoenix → Browser
Version: unspecified → Trunk
Reproduced with 2003010505-trunk/Linux. Confirming bug.
Severity: normal → major
Status: UNCONFIRMED → NEW
Ever confirmed: true
hmmm. http://www.w3.org/TR/html4/struct/objects.html#h-13.6.2 says to append
?x,y to the specified url.

that would mean that AddNote.php?coord=?x,y is the correct thing. however...
that is no valid url...
Assignee: asa → other
Component: Browser-General → Layout
QA Contact: asa → ian
Confirming 2003010508/MacOS 9.2.2.
AddNote.php?coord=?x,y is a valid uri, I think.

http://www.ietf.org/rfc/rfc2396.txt
absoluteURI   = scheme ":" ( hier_part | opaque_part )
hier_part     = ( net_path | abs_path ) [ "?" query ]
query         = *uric
uric          = reserved | unreserved | escaped
reserved      = ";" | "/" | "?" | ":" | "@" | "&" | "=" | "+" |
                "$" | ","

1.2.1 doesn't have this problem.
Regression.
But section 2.2 explains why those characters are allowed:

# These characters are called "reserved", since their usage within the URI 
# component is limited to their reserved purpose.  If the data for a URI component
# would conflict with the reserved purpose, then the conflicting data must be 
# escaped before forming the URI.

Since "?" would conflict with its reserved purpose in this case, it must be escaped.
I see. But we cannot escape "?x,y".
In the case of server-side image map, URI specified by the href
must not contain query?
basically, the issue is:
the HTML spec just says to append ?x,y to the specified HREF.
the URI RFC forbids urls with ?coord=?x,y

my personal opinion is that the best thing to do is what mozilla does now...
> Since "?" would conflict with its reserved purpose in this case, it 
> must be escaped.

I don't understand that "?" conflicts with its reserved purpose in this case.
The spec says:
> hier_part     = ( net_path | abs_path ) [ "?" query ]
but neither net_path nor abs_path can include "?".
Therefore I suppose it clear that a query follows the first question mark, not
the second.

I agree with comment 8.

Another example (Japanese page):
http://mapbrowse.gsi.go.jp/cgi-bin/nph-mm.cgi?mesh=5339461&x=-1&y=-1
This is a map near the Tokyo staion.
You will see "Fail to find position parameters." on Mozilla lately
when clicking somewhere on the map.
gsi.go.jp is National Geographical Survey Institute in Japan.
*** Bug 190776 has been marked as a duplicate of this bug. ***
Part 3.4 of RFC 2396 give a direct statement: 

   Within a query component, the characters ";", "/", "?", ":", "@",
   "&", "=", "+", ",", and "$" are reserved.

So usage of ?coords=?x=10,y=10 is incorect.
The problem here is that the HTML spec is a little inconsistant; on one hand '?'
cannot appear in the query part of the URI and on the other hand the spec
clearly states that "?x,y" is what must be appended to the href.

I can see exactly one argument in favor of stripping the query from the href
string, namely that the result is not a valid uri, basicly nitpicking IMHO,
however there are several good arguments for not messing with the users data:

1) The spec clearly states that "?x,y" must be appended, so it is clearly
   against the spec to mangle the href string before doing that.

2) Every browser on the planet (well more or less) is compliant with the
   spec on this point, so violating the spec on purpose is stupid.

3) Following the spec to come up with a technically invalid uri is not
   the browsers fault it is the users, so the browser should not try to
   be smart and simply do as it's told, it doesn't mangle (or bitch about)
   a url like this one: http://somewhere/page?param=?.

In short: Stop being such a smartass and revert to the old (and HTML4 compliant)
behavior, we'll change to using
 <form ...><input type="hidden" ...><input type="image" ismap></form>
when we are good and ready, not as a special workaround for a b0rken browser.
Priority: -- → P4
Target Milestone: --- → Future
*** Bug 192224 has been marked as a duplicate of this bug. ***
Bug 190891 is closely related to this.
That case is <form action="foo?foo">.
This should be changed to All/All.
*** Bug 196672 has been marked as a duplicate of this bug. ***
Keywords: regression
Hardware: PC → All
Summary: href mis-handled on image map (worked in 0.4) → server-side imagemap replaces URL query rather than appending
I agree with #12 - the HTML spec just say's to append ?x,y to the URL specified
in the surrounding A tag's href attribute. Deciding whether or not a string is a
valid URL should be the servers job, not the browsers, and if a server allows an
invalid URL to return something meaningful, then it's their error, not ours. We
should be sending what the web page tells us to send.

Another example - the much used www.streetmap.co.uk which is now broken due to
this non-standards-compliant 'cleverness'.
Flags: blocking1.3+
Shouldn't have set 1.3 flag to +. Have reset to ? as I think this is an
important bug since it breaks standards compliance.
Flags: blocking1.3+ → blocking1.3?
When did this regress?  If it's old as the hills, not a new regression since
1.2.1, we are not going to hold 1.3 for it.  At this point, we probably won't
hold 1.3 for anything that isn't a very likely crash or dataloss, and a
regression from 1.2.1.  Anyway, that's my reading of drivers' sentiments.

/be
No - this is a regression since 1.2.1. In 1.2.1 the coordinates are handled
properly, and www.streetmap.co.uk (for example) works as expected.
Does 1.3 actually have this problem?  On WinXP 2003030408, I can't reproduce. 
Steps:

1. Go to http://www.watersheep.org/postit/
2. Click on some of the gray area
3. URL shows as http://www.watersheep.org/postit/AddNote.php?518,264

Cool app, BTW.
My 4 or 5 day old 1.3 build doesn't have the problem either.  I suspect it's
gone.  Can you please retest on a more recent version or help me out with
different instructions?

And yes, I believe stripping ?coords= would be the right thing.  We do that when
we do other GET requests and IIRC that's not specified either.
> And yes, I believe stripping ?coords= would be the right thing.  We do that when
> we do other GET requests and IIRC that's not specified either.

Unfortunately, it's not what the HTML spec says, it's not what other browsers
do, and it's not the behavior that web sites depend on.

The behavior you're describing is the bug.
OK, this was introduced in bug 176904,
http://bonsai.mozilla.org/cvsblame.cgi?file=mozilla/layout/html/base/src/nsImageFrame.cpp#1756

Should be simple enough.
Assignee: other → jkeiser
Attached patch Patch — — Splinter Review 
This fixes the problem.  The earlier checkin introduced the behavior of just
setting the query string, which obliterates any earlier query string.  An
alternate fix might be to append to the query string in the QI'd case, but that
seems like a waste of code.  This is simple and direct.
Attachment #117012 - Flags: superreview?(bzbarsky)
Attachment #117012 - Flags: review?(peterl)
Flags: blocking1.3? → blocking1.3+
Comment on attachment 117012 [details] [diff] [review]
Patch

<sigh>.  What a mess.
Attachment #117012 - Flags: superreview?(bzbarsky)
Attachment #117012 - Flags: superreview+
Attachment #117012 - Flags: review?(peterl)
Attachment #117012 - Flags: review+
Attachment #117012 - Flags: approval1.3?
Fixed on trunk.  Leaving open until 1.3 approval is settled.
Status: NEW → ASSIGNED
Comment on attachment 117012 [details] [diff] [review]
Patch

This seems to have missed the 1.3 train.  Perhaps for 1.3.1, though, if we do
such a thing.
Attachment #117012 - Flags: approval1.3+ → approval1.3-
Leaving open for 1.3.1.
*** Bug 197314 has been marked as a duplicate of this bug. ***
quote: "Deciding whether or not a string is a valid URL should be the servers 
job, not the browsers, and if a server allows an invalid URL to return 
something meaningful, then it's their error, not ours."

reply: Except that the (unofficial) DOM Level 0 requires the browser to be 
able to parse the query string from the URL.  Why shouldn't we post a question 
to USENET newsgroup comp.infosystems.www.authoring.html or the technical 
discussion list www-html@w3.org (in fact, I might just do that)?
Whiteboard: 1.3.1
*** Bug 197775 has been marked as a duplicate of this bug. ***
I started a discussion at comp.infosystems.www.authoring.html about this 
issue.  You can read about it here: http://groups.google.com/groups?
dq=&hl=en&lr=&ie=UTF-8&oe=UTF-
8&th=48c61f72e3fc0eb8&seekm=Xns9343539E54B6Bjkorpelacstutfi%
40193.229.0.31&frame=off .  So far (from one responce) it mostly confirms John 
Keiser's patch.  Will this affect the JavaScript processor at all?
*** Bug 199089 has been marked as a duplicate of this bug. ***
Checked in to 1.3.1.
Status: ASSIGNED → RESOLVED
Closed: 21 years ago
Resolution: --- → FIXED
Whiteboard: 1.3.1 → fixed1.3.1
*** Bug 190891 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.

Attachment

General

Creator:
Created:
Updated:
Size: