221445 - Using "|" (pipe) as URI separator breaks some links/home pages; causes multiple tabs to open

Status: RESOLVED FIXED

(Firefox :: Tabbed Browser, defect, P3)

Description

[Mike Connor [:mconnor]]

Discovered initially by someone trying to check mail from yahoo messenger, but
the login URL uses two | characters, so if Firebird is not open, it breaks this
into three tabs.  Since | is a valid character in URLs, we can't exactly blame
yahoo here.

Either we need to send multiple homepage URLs to multiple prefs,  with an
integer pref for how many are currently set (since if we go from 4 to 3, we
couldn't delete browser.homepage.URL.4, just limit the loop to the number of
active homepages) or we need to come up with a different separator that's not a
valid URI character and can still be passed to Firebird on startup, however I'm
not sure if we need to support multiple pages from external apps/command line

The workaround is to open Firebird before checking mail.

Note to people cced on the bug, bug 217370 has been obsoleted in favour of a
clean bug.

Comment 1

[alanjstr]

You couldn't just wait for someone to clean the other one and then reopen it?

Comment 2

[Mike Connor [:mconnor]]

Asa asked that a new bug be created and moved the bug to another group to make
it really really inaccessible.  Guess that's easier and less risky than trying
to use a SQL statement to edit one field in the database

Comment 3

[alanjstr]

justdave said he fixed it.  Its not unlocked yet, though.  

Comment 4

[Dave Miller [:justdave]]

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

Comment 5

[alanjstr]

Can we just dissallow pipe seperated URLs becoming multiple tabs when they're
from the outside?

Comment 6

[Mike Connor [:mconnor]]

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

Comment 7

[Jason Barnabe (np)]

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

Comment 8

[Mike Connor [:mconnor]]

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

Comment 9

[Mike Connor [:mconnor]]

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

Comment 10

[ruvinsky]

When is this bug planned to be fixed?  Perhaps in the 0.9 timeframe?

Comment 11

[Mike Connor [:mconnor]]

this is a problem with yahoo IM integration, we should look at changing how we
handle this

putting on 1.0 radar for now

Comment 12

[Mike Connor [:mconnor]]

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

Comment 13

[Mike Connor [:mconnor]]

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

Comment 14

[Ben Goodger (use ben at mozilla dot org for email)]

Of all the bogus characters to pick as a delimiter. 

Sigh. 

Comment 15

[Horatiu Cristea]

the bug persists also on firefox 0.9RC. (Mozilla/5.0 (Windows; U; Windows NT
5.0; en-US; rv:1.7) Gecko/20040608 Firefox/0.8)

Comment 16

[Aaron Kaluszka]

Re comment 14: Yeah, especially since a lot of sites started using | instead of
& to avoid the & change of standard (another unfortunate choice of delimiter).

Comment 17

[chris hofmann]

mconnor, could you have a look at trying to come up for a fix for this?

Comment 18

[Steffen Wilberg]

I guess space is a good alternative.

http://www.ietf.org/rfc/rfc2396.txt says:
2.4.3. Excluded US-ASCII Characters
   The space character is excluded because significant spaces may
   disappear and insignificant spaces may be introduced when URI are
   transcribed or typeset or subjected to the treatment of word-
   processing programs.  Whitespace is also used to delimit URI in many
   contexts.
   space       = <US-ASCII coded character 20 hexadecimal>

Comment 19

[alanjstr]

If you use space, would that cause a problem with "c:\program files" ?

Comment 20

[Steffen Wilberg]

> If you use space, would that cause a problem with "c:\program files" ?
No, that would be replaced by file:///c:/program%20files/.

Taking, patch upcoming.

Comment 21

[Steffen Wilberg]

Attachment: patch

Using " " instead of "|" is quite simple, but migrating old prefs is not.

migrateHomepageDelimiter() is called on each startup, before the homepage(s)
load. The pref is migrated if the new pref
"browser.startup.homepage.delimiter.migrated" is not set to true (or doesn't
exist).

I moved the assignment of gPrefService up quite a bit from delayedStartup()
into Startup(), since gHomeButton.getHomePage() needs that, which I call in
migrateHomepageDelimiter().

selectBookmark.xul is only used to set the homepage, as you can see in the
title of that dialog.

Comment 22

[Jesse Ruderman]

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

Comment 23

[Robert]

My list mail application uses | in the links in HTML email messages to do basic link tracking.

When clients who have Firefox click the links in my emails (from an email client like Outlook for 
example), Firefox launches and opens 13+ tabs and opens seemingly random pages in each tab.

This bug us giving the appearance that my opt-in email list is some sort of SPAM invasion that takes 
over the users browser when they click the links in my emails.

Comment 24

[Robert]

This bug still has not been fixed as of the new 1.0 release. What gives?

Comment 25

[Aaron Kaluszka]

That was the 1.0PR release.  Notice that this bug is marked
blocking-aviary1.0PR-  It is, however, marked blocking-aviary1.0+ so you'll just
have to be patient and refrain from spamming.

Comment 26

[Ben Goodger (use ben at mozilla dot org for email)]

Comment on attachment 155080 [details] [diff] [review]
patch

Is it ever the case that there are spaces in a homepage value that are valid?
I.e. is this list escaped for display? I think using spaces might become
confusing.

Comment 27

[alanjstr]

Spaces would be a problem if someone is trying to load a local file and the path
has spaces in it, no?

Comment 28

[Steffen Wilberg]

Attachment: unbitrotted

Comment 29

[Steffen Wilberg]

Comment on attachment 160367 [details] [diff] [review]
unbitrotted

(In reply to comment #26)
> Is it ever the case that there are spaces in a homepage value that are valid?
No. Spaces are not allowed at all in URIs, see comment 18.

> I.e. is this list escaped for display? I think using spaces might become
> confusing.
In Options-General, multiple homepages are currently displayed as e.g.
    http://www.mozilla.org/|file:///C:/copy%20of%20test.html
With my patch applied, it displays:
    http://www.mozilla.org/ file:///C:/copy%20of%20test.html
Looks better, doesn't it?

(In reply to comment #27)
> Spaces would be a problem if someone is trying to load a local file
> and the path has spaces in it, no?
No. Spaces in filenames are escaped by %20, e.g.
file:///C:/copy%20of%20test.html

Comment 30

[Steffen Wilberg]

Attachment: unbitrotted, v2

Comment 31

[alanjstr]

> (In reply to comment #27)
> > Spaces would be a problem if someone is trying to load a local file
> > and the path has spaces in it, no?
> No. Spaces in filenames are escaped by %20, e.g.
> file:///C:/copy%20of%20test.html

What I mean is: if someone wants to launch multiple files from the command line,
how do they separate them? 
C:\My Docs\launch1.html C:\My Docs\launch2.html
Or are you assuming that anything/anyone that passes a filename in will have
already escaped it properly for you?

Comment 32

[Steffen Wilberg]

> What I mean is: if someone wants to launch multiple files from the
> command line, how do they separate them? 
Ok, that's a problem indeed.
   firefox "C:\My Docs\launch1.html"
works fine (I tested the equivalent on Linux, using an absolute path), but
   firefox "C:\My Docs\launch1.html C:\My Docs\launch2.html" doesn't.

We could keep using the pipe for external URIs and only change the homepage
string to spaces. But that wouldn't fix links from external applications like
Yahoo Messenger, and that's what most of the dupes are about.

Another problem with my patch is that
   firefox "www.mozilla.org www.mozillazine.org"
doesn't work on Linux anymore if Firefox is already running. It says:
   ./firefox: line 166: [: too many arguments
   ./mozilla-xremote-client: Error: Failed to send command: 509 internal error

I'm not sure if we should use one of these characters instead:
   <  >  "
"The angle-bracket "<" and ">" and double-quote (") characters are
   excluded because they are often used as the delimiters around URI in
   text documents and protocol fields."
http://www.ietf.org/rfc/rfc2396.txt

Comment 33

[Ben Goodger (use ben at mozilla dot org for email)]

They may not be allowed, i.e. escaped in the requests sent to servers, but there
are links with spaces all the time (e.g. look at the installer download link on
our website), and people can type in file names with spaces into the home page
setting field on the options panel

The safest option is to do what Seamonkey does and use a series of prefs like this:

browser.startup.homepage.1
browser.startup.homepage.2
browser.startup.homepage.3 ...

but in that case you *must* migrate over the old ones one time. 

Comment 34

[alanjstr]

I think I can live with enclosing earch URI with angle brackets if more than one
is specified.  But for a single URI it shouldn't need them (but handle it if
they're there).

Comment 35

[Phil Ringnalda (:philor)]

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

Comment 36

[Aaron P]

An alternative could be to use 2 sequential pipe characters "||" to separate
URIs. That way, links that use | as a separator would work fine. Obviously, this
would mean that URIs with "||" in them wouldn't work, but I don't think anyone
does this.

The only remaining issue would be backwards compatibility - porting old
multi-tab bookmarks that still use "|" to separate the URIs. However, that could
be addressed in another bug.

Comment 37

[Phil Ringnalda (:philor)]

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

Comment 38

[Bill Mason]

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

Comment 39

[Anbo Motohiko]

This issue is annoying thing for some Japanese Windows users.
When they open a local file which path contains certain characters
from explorer, Firefox does not open it.

Some of the characters causing this are:
\u6383,
\u5f13,
\u82b8,
\u92fc,
...

Japanese Windows uses SHIFT_JIS locale. When these characters used
in paths, encoded URIs will contain '|'(\u007c).
e.g., "c:\\u6383\" will be "c:\%91|\".


This does not occurs in Application Suite.

Comment 40

[Mike Connor [:mconnor]]

Steffen, are you still working on this?  We have some basic stuff for storing
multiple homepages that I think is old seamonkey stuff, or its skeletonized
stuff that was never finished.  Ping me on IRC if you want to know more, or let
me know and I'll try to get this done myself.

Comment 41

[Steffen Wilberg]

Sorry, I have very little time at the moment. -> mconnor per comment 40.

Comment 42

[Mike Connor [:mconnor]]

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

Comment 43

[Jungshik Shin]

(In reply to comment #39)
> This issue is annoying thing for some Japanese Windows users.
> When they open a local file which path contains certain characters
> from explorer, Firefox does not open it.

> Japanese Windows uses SHIFT_JIS locale. When these characters used
> in paths, encoded URIs will contain '|'(\u007c).
> e.g., "c:\\u6383\" will be "c:\%91|\".

 What happens if you try to open one of those files on the command line (that
is, 'firefox.exe "file://c:/....../....' in Start | Run)? 

Comment 44

[patrice.grometto]

(In reply to comment #25)
> That was the 1.0PR release.  Notice that this bug is marked
> blocking-aviary1.0PR-  It is, however, marked blocking-aviary1.0+ so you'll just
> have to be patient and refrain from spamming.

firefox 1.0.3 still does not display those urls correctly (example
http://www1.indexel.net/visu_campagne.jsp?SID=161|1|24831 )
; multiple tabs are opened... 

Comment 45

[Matt]

Hey, how about putting each homepage link within quotes?  You can have spaces
automatically converted to %20 when inside quotes, and literal quotes aren't
allowed in URI's anyways.

Example:
"http://www.google.com/" "https://gmail.google.com/gmail" "http://my.yahoo.com/"

But for one URL, it isn't necessary to put quotes (they would be implied), but
they can be put in.  If there is ever a need to put quotes into the URI, just
backslash them:
"file:///c:/documents and settings/john/my documents/\"Home\" Page.html"

Also, when "grepping" the string, you can do it in such a way that it doesn't
matter what's in between the quotes.

To check for errors, when a user changes their bookmarks, do some sort of check
on it to make sure there aren't any unmatched quotes, and maybe even have some
sort of dialog that says:
"You are about to change your homepage(s) to the following:
http://www.google.com/
https://gmail.google.com/gmail
http://my.yahoo.com/
If this is *not* correct, please cancel and check your homepage(s) for any
unmatched quotes (")."

I'd say this could be one of those 1.8b2 or 1.8b3 kind of things.  I might be
able to make a patch as well.

Comment 46

[alanjstr]

(In reply to comment #45)
> Hey, how about putting each homepage link within quotes?  
Wouldn't that be a problem passing stuff from the command line?

Comment 47

[Matthew van Eerde]

What about javascript: url's - those could contain any number of wacko characters?

For the command line, it seems to make sense to pass separate URLs as separate
command-line arguments.  Enclose "C:\Program Files" in quotes in the normal
shell fashion.

As far as a pref, I like the idea of one-pref-per-URL.

Comment 48

[Jesse Ruderman]

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

Comment 49

[Dave Townsend [:mossop]]

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

Comment 50

[Phil Ringnalda (:philor)]

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

Comment 51

[Bruce Pascoe]

I actually like the idea of one-pref-per-homepage.  It would also make the
preferences dialog more user friendly.  There could be a seperate "home pages"
tab, and from there you could add and remove homepages in much the same fashion
as the "allowed sites" dialog box.  As for opening multiple tabs from the
command line, I really don't see what the big issue is--use one argument per URL
or path.  If an URL or path contains spaces, place it it quotes.  Now, I'm
seeing people here complaining about the following command line not working:
firefox "C:\test file 1.html C:\test file 2.html"

...which it wouldn't (how would you know where to seperate the filenames?) 
However, the following would:
firefox "C:\test file 1.html" "C:\test file 2.html"

So what's the big deal?  I'm sure there's a method for each OS to get the ENTIRE
argument string, rather than one parameter at a time.  Then all you have to do
is parse that string as a whole.

Comment 52

[Daniel Veditz [:dveditz]]

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

Comment 53

[Adam Guthrie]

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

Comment 54

[Matthew Sackman]

(In reply to comment #51)
> If an URL or path contains spaces, place it it quotes.  Now, I'm
> seeing people here complaining about the following command line not working:
> firefox "C:\test file 1.html C:\test file 2.html"
> ...which it wouldn't (how would you know where to seperate the filenames?) 

Absolutely, you can't replace firefox in the above with rm and expect it to work so why should firefox try to be smarter / violate x years of expected behaviour of applications on the command line?

The proposed solution sounds ideal: the homepage prefs gets reworked to loose the | delimiter so then all | parsing can be dropped and then command line arguments get treated as one URL per arg.

Comment 55

[Emil Stenström]

This bug still annoying people in Firefox 1.5. 

http://www.jmedata.se/start.asp?store=I&target=product.asp||dept_id=||itemno=24321

That URL, when opened from external program, opens in five tabs, none which display the correct webpage. All links that are opened from clicking links in external applications get this faulty behaviour so I think this fix is pretty important.

Also, adding "pipe" and "multiple tabs" to the summary of this bug might make it easier to find, thus producing fewer duplicate bug reports on this one.

Comment 56

[DAgy]

Just disable that tabbing stuff damn !
For me this has no sense and no real usage.
When firefox is not started yet I've got multiple tabs. But when allready running
I've got correct url. I'm asking... WHAT IS THIS GOOD FOR ????

Comment 57

[B.J. Herbison]

(In reply to comment #56)
> Just disable that tabbing stuff damn !
> For me this has no sense and no real usage.

The key phrase there is "For me".  A friend of mine got excited about Firefox partially because of the ability to open multiple tabs at startup.

However, an option to disable the | parsing might make some people happier until a long-term solution can be found for this bug.

Comment 58

[Mike Connor [:mconnor]]

bug 318088 moved with Places, but we need a fix for this in b1, this is a bugfix

Comment 59

[KengHoe Tan]

!?

Comment 60

[Mike Connor [:mconnor]]

this should depend on the prefwindow rewrite now

Comment 61

[Mike Connor [:mconnor]]

pushing out non-critical-path bugs to b2

Comment 62

[Michael Ventnor]

Would a fix for this also affect bug 343999 ?

Comment 63

[Adam Guthrie]

(In reply to comment #62)
> Would a fix for this also affect bug 343999 ?

Depends on the scope of the fix for this bug, of course (but I kinda doubt it).

Comment 64

[Aaron P]

How about making it so that if you want to use the pipe as a URI separator, the FIRST character in the URI would have to be a pipe? Conversely, if you do not start the URL with a pipe character, it would assume that any pipes found are part of a single URL. 

This would solve most test cases. There are only two instances that this would not solve:
1) When you want to open multiple tabs where one of those tabs has a URI containing a pipe character. 
2) When your URL actually starts with a pipe character -- which is impossible except in cases where you have relative links pointing to files.

For instance, to open two sites in tabs, you would use something like:
|site1.com|site2.com
or 
|http://site1.com|http://site2.com

Comment 65

[Mike Connor [:mconnor]]

This isn't going to make Fx2, unfortunately, but will be easily fixed for Places.

Comment 66

[Michael Lelli]

As I see it, the seperator should be one of the keys on 104-key PC US English QWERTY keyboards, one of the most common keyboard layouts. However, that leaves us with only four characters to use: Tab, Space, Spacing Grave/Grave Accent (`), and Backslash (\). Since Tab is used for navitaging in non-wordproccor programs and web servers running on Windows can use the Backslash for seperating directories, those two are out. I also don't like the idea of using the Spacing Grave for it, for somewhat obvious reasons.

The only way I see it is using a space, and for people using spaces in their homepage URLs, we can convert them to "%20", and if we really need to, "+" in search queries before replacing the pipe characters. Since my computer is very full right now, I don't have the mozilla source and will take a little time for me to make the patch.

Comment 70

[Mike Connor [:mconnor]]

moving out bugs that don't need to block b1

Comment 71

[Jaroslav Zaruba]

how about new-line as the separator?

Comment 72

[Asa Dotzler [:asa]]

Indeed, like IE 8, we should have a multi-line field for that and put one URL on each line.  

Comment 73

[Thomas D. (:thomas8)]

Maybe when this bug lands, the following, related RFE could be considered, too?

Related Bug 421785 (cannot use bookmarks to open multiple URLs) asks for the implementation of multiple URL handling not just for program startup or browser start page, but also for bookmark URL. So that a single click on a bookmark will open multiple tabs with different URLs.

Maybe someone can add Bug 421785 to the depends-on or blocks list.

Comment 74

[Daniel Veditz [:dveditz]]

Thomas D.:  Bookmarks already have multiple-URL handling. Put the bookmarks in a folder and then open the folder. Normal click brings up a sub-menu with a "Open all in tabs" item, control-click on the folder just does it.

Comment 75

[Mike Connor [:mconnor]]

This isn't a blocker, maybe ever, but we should fix it at some point soon...

Comment 76

[[:Cww]]

Should we link this to bug 318088 somehow?

Also as a temporary workaround, can we use " | " as a separator as in pad with spaces (which I always thought was the official way of doing it anyhow)?

Comment 77

[:Gavin Sharp [email: [e-mail]]]

This is FIXED by bug 441120, as far as I can tell. URLs containing pipes are no longer split when passed in through the command line.

Comment 78

[:Gavin Sharp [email: [e-mail]]]

(We might still want to change the way we store multiple home pages in the future, but that can happen in a new bug.)

Comment 79

[Jesse Ruderman]

I reopened bug 244192 for that.