29256 - Illegal character in window name warning on '-' (nsWindowWatcher::CheckWindowName is too strict)

Status: RESOLVED FIXED

(Core :: DOM: Core & HTML, defect)

Description

[Suresh Duddi (gone)]

I hit this assertion when browsing. The character is question is the minus in
"current-data"

Looks like we dont like the window name to have a '-' That sounds bogus.

(gdb) bt 20
#0  0x404c34e1 in ?? () from /lib/libc.so.6
#1  0x404c4868 in ?? () from /lib/libc.so.6
#2  0x400de0cb in JS_Assert (s=0x400e6370 "reportp", file=0x400e61ea "jsexn.c",
ln=549) at jsutil.c:146
#3  0x4008913c in js_ErrorToException (cx=0x81bebf8, message=0x887aaa8 "illegal
character '-' ('\\55') in window name current-data", reportp=0x0) at jsexn.c:549
#4  0x4006b5be in ReportError (cx=0x81bebf8, message=0x887aaa8 "illegal
character '-' ('\\55') in window name current-data", reportp=0x0) at
jscntxt.c:260
#5  0x4006b727 in js_ReportErrorVA (cx=0x81bebf8, flags=0, format=0x403ea040
"illegal character '%c' ('\\%o') in window name %s", ap=0xbfffed60) at
jscntxt.c:309
#6  0x40064f14 in JS_ReportError (cx=0x81bebf8, format=0x403ea040 "illegal
character '%c' ('\\%o') in window name %s") at jsapi.c:2970
#7  0x4032bd34 in GlobalWindowImpl::CheckWindowName (this=0x81de500,
cx=0x81bebf8, aName=@0xbffff084) at nsGlobalWindow.cpp:2738
#8  0x40329a28 in GlobalWindowImpl::OpenInternal (this=0x81de500, cx=0x81bebf8,
argv=0x88789c8, argc=4, aDialog=1, aReturn=0xbffff31c) at
nsGlobalWindow.cpp:2328
#9  0x403258f1 in GlobalWindowImpl::OpenDialog (this=0x81de500, cx=0x81bebf8,
argv=0x88789c8, argc=4, aReturn=0xbffff31c) at nsGlobalWindow.cpp:1519
#10 0x40ed2e1c in ?? () from
/home/dp/build.debug/mozilla/dist/bin/components/libmozbrwsr.so
#11 0x40cef9eb in ?? () from
/home/dp/build.debug/mozilla/dist/bin/components/liburiloader.so
#12 0x40cedabf in ?? () from
/home/dp/build.debug/mozilla/dist/bin/components/liburiloader.so
#13 0x40ced779 in ?? () from
/home/dp/build.debug/mozilla/dist/bin/components/liburiloader.so
#14 0x41a2fb61 in ?? () from
/home/dp/build.debug/mozilla/dist/bin/components/libnecko_cache.so
#15 0x419cb8c6 in ?? () from
/home/dp/build.debug/mozilla/dist/bin/components/libnecko_http.so
#16 0x419cae26 in ?? () from
/home/dp/build.debug/mozilla/dist/bin/components/libnecko_http.so
#17 0x406e4ae1 in ?? () from
/home/dp/build.debug/mozilla/dist/bin/components/libnecko.so
#18 0x406e3ef7 in ?? () from
/home/dp/build.debug/mozilla/dist/bin/components/libnecko.so
First symbol in segment of executable not a source symbol
(gdb)

Comment 1

[vidur (gone)]

window.open issue. Trivial fix, but I thought I'd pass it along anyway. :-)

Comment 2

[sean echevarria]

This can be reproduced by clicking on the blue "Real" logo bubble on the Real 
Player plugin.  Another character that is accepted in other browsers in window 
open target names and isn't in moz is ' ' (space).  Possibly also '!'.

Comment 3

[Dan M]

  Mozilla certainly explicitly disallows window names with any but alphanumeric 
characters, or a leading underscore, though that's sort of a reserved namespace. 
It turns out Navigator 4 does allow an embedded hyphen. IE4 doesn't. That's 
surprising. I thought hyphens were illegal. What browsers are you running that 
allow bangs and spaces?
  I suppose we should allow hyphens like Navigator.

Comment 4

[sean echevarria]

Goto the following url and rt-click on the plugin.  From the popup menu 
that appears, select the Copyright menu item.  Click on the url that appears in 
the copyright dialog.  This will open up a new browser window whose target name 
has a space.  It works in navigator 4.7.

http://www.beatnik.com/authoring/products/music-object/documentation/tutorials/p
utting-music-on-a-page/steps/a-simple-embed.html

I don't have an example for bang (I only mentioned it because I came across 
now unused code in the Beatnik plugin that used a bang in a window target and 
do recall that particular feature working correctly some time ago).

Comment 5

[Peter Trudelle]

Mass-moving all M16 non-feature bugs to M17, which we still consider to be 
part of beta2

Comment 6

[Hugh Kennedy]

*** Bug 35444 has been marked as a duplicate of this bug. ***

Comment 7

[Hugh Kennedy]

this also shows up in M14 on WinNT and Solaris 2.6 (from bug 35444.)
marking os and plat as "All"

Comment 8

[Peter Trudelle]

moving to m18

Comment 9

[Peter Trudelle]

mass-moving all bugs to m21 that are not dofood+, or nsbeta2+

Comment 10

[Dan M]

moving en masse all bugs that didn't get nsbeta3 nomination to "future" milestone

Comment 11

[Andreas Kunz]

Ummm... no comments for more than three years and the URL is 404... does this
still apply?

Comment 12

[Dan M]

This isn't really an issue any longer. What was a JavaScript exception at the
time this bug was filed is now just a warning printed to stdout, and only on
debug builds. End users would never see the warning.

I just browsed half a dozen JS references on the web. None mentioned any hard
restrictions to the character set of a window name, though one did suggest them.
(Random published JS tutorials are pretty much the best reference available for
DOM 0 features.) A window opened with an illegal name is still opened; it's just
that debug Mozilla complains about it. And code like this

const LEGALNAME = 'aw';
const BADNAME = 'aw $x!';
var hurtMe = window.open('about:blank', LEGALNAME);
hurtMe.name = BADNAME;
window.open('http://www.mozilla.org', BADNAME);

also works fine in Mozilla (window hurtMe displays mozilla.org), with never a
warning given.

I say kill off the window name check. It has no teeth, and no one's listening.

Comment 13

[Dan M]

Note that IE6 works similarly, though it's a little flakier. However it throws a
real script exception when confronted with an attempt to open a window given a
name containing spaces or bangs.

So at the moment we are less restrictive than IE. We could reasonably remove the
name check altogether or even restore the JS exception. But the current warning,
a watered-down version of the JS exception, is more or less correct.

Comment 14

[Boris Zbarsky [:bzbarsky]]

I think I would be in favor of removing the warning, especially as it does all
sorts of unnecessary work in opt builds...

Comment 15

[Serge Gautherie (:sgautherie)]

Filter on "Nobody_NScomTLD_20080620"

Comment 16

[gekacheka]

nsWindowWatcher::CheckWindowName is too strict, gives off spurious warnings in debug builds (e.g., bug 257942 comment 41, bug 258480, bug 332442, bug 393488 comment 26, bug 462837, bug 461574, bug 507700).
It should accept frame-target names.  Below is some research that may help a C++ developer.


CURRENT BEHAVIOR

It currently only accepts ASCII alpha, ASCII digit, or '_', all in any position in name.  
(Accepts "123" as a name, but not "dialog.options.view", "help-window", nor "http://example.com/site-help".)
  1394 for (aName.BeginReading(scan); scan != endScan; ++scan)
  1395     if (!nsCRT::IsAsciiAlpha(*scan) && !nsCRT::IsAsciiDigit(*scan) &&
  1396         *scan != '_') {
  [http://mxr.mozilla.org/mozilla-central/search?string=nsWindowWatcher::CheckWindowName]

CVS blame dates the current code to 2001-02-16 by bug 67368, but that postdates bug 29256 from 2000-02-25, so the code is probably older and copied from elsewhere (bug 29256 was originally about an assertion).  (Speculation: Maybe dates to HTML 3.2 [1997] or earlier, where the syntax of acceptable names was not easily discerned, so warning may have been based on what other browsers did then.  If so, needs updating to HTML 4.01.)


HTML SPECIFICATIONS

HTML 4.01 SGML Basic Types
  * CDATA ... may include character entities ... replace entities with
    characters ...  user agents may ignore leading and trailing whitespace
  * ID and NAME tokens must begin with a letter ([A-Za-z]) and may be
    followed by any number of letters, digits ([0-9]), hyphens ("-"), 
    underscores ("_"), colons (":"), and periods (".").
  [http://www.w3.org/TR/html401/types.html#h-6.2]

HTML 4.01 16.3 Specifying Target Name
  [http://www.w3.org/TR/html401/present/frames.html#h-16.3]

HTML 4.01 6.16 Frame target names
  says a frame-target name must start with an alphabetic character
  [a-zA-Z] except for the special names that start with
  _ (_blank, _self, _parent, _top).
  [http://www.w3.org/TR/html401/types.html#type-frame-target]

  The DTD says FrameTarget is CDATA.

If there is a reason not to allow just the special names or aribitrary CDATA that starts with an ascii letter, then since it is called a name, and appears in the frame name attribute, then it would be consistent to warn using the same restrictions as on the anchor name attribute.  (Assume entities are replaced with characters before reaching here.)

HTML 4.01 12.2.1 Syntax of anchor names
  * Anchor names should be restricted to ASCII characters
  [http://www.w3.org/TR/html401/struct/links.html#h-12.2.1]
HTML 4.01 12.2.3 Anchors with the id attribute
  * id and name share the same namespace
  * when both values are used on a single element,
    their values must be identical
  * the name attribute may contain character references
    (examples include names entities for non-ascii characters)
  [http://www.w3.org/TR/html401/struct/links.html#h-12.2.3]


POSSIBLE IMPROVED BEHAVIORS

Option 1: warn if not a name token (allowed as id), 
  or starts with _ but not special token (_blank, _self, _parent, _top)
  /([a-zA-Z][-.:_0-9A-Za-z]*)|_blank|_self|_parent|_top/
    
Option 2: warn if does not start with an ascii letter (A-Za-z)
  or starts with _ but not special token (_blank, _self, _parent, _top)
  or contains newlines
  /([a-zA-Z].*)|_blank|_self|_parent|_top/

Option 3: eliminate warning even from debug builds


(I hope this helps.)

Comment 17

[:Ms2ger (he/him; ⌚ UTC+1/+2)]

Attachment: Patch v1

Comment 18

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 539175 [details] [diff] [review]
Patch v1

r=me

Comment 19

[:Ms2ger (he/him; ⌚ UTC+1/+2)]

http://hg.mozilla.org/mozilla-central/rev/779f9476693c