Closed Bug 221445 Opened 17 years ago Closed 12 years ago

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

Categories

(Firefox :: Tabbed Browser, defect, P3, minor)

defect

Tracking

()

RESOLVED FIXED
Firefox 3.1a1

People

(Reporter: mconnor, Assigned: Gavin)

References

(Blocks 1 open bug)

Details

(Keywords: intl, top100)

Attachments

(3 obsolete files)

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.
You couldn't just wait for someone to clean the other one and then reopen it?
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
justdave said he fixed it.  Its not unlocked yet, though.  
*** Bug 217370 has been marked as a duplicate of this bug. ***
Can we just dissallow pipe seperated URLs becoming multiple tabs when they're
from the outside?
*** Bug 228690 has been marked as a duplicate of this bug. ***
*** Bug 234215 has been marked as a duplicate of this bug. ***
*** Bug 238911 has been marked as a duplicate of this bug. ***
*** Bug 239327 has been marked as a duplicate of this bug. ***
When is this bug planned to be fixed?  Perhaps in the 0.9 timeframe?
this is a problem with yahoo IM integration, we should look at changing how we
handle this

putting on 1.0 radar for now
Flags: blocking1.0?
Summary: Using "|" as URI separator breaks some links, including yahoo mail → Using "|" as URI separator breaks some links/home pages
*** Bug 244192 has been marked as a duplicate of this bug. ***
*** Bug 244346 has been marked as a duplicate of this bug. ***
Of all the bogus characters to pick as a delimiter. 

Sigh. 
Flags: blocking1.0? → blocking1.0+
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)

Flags: blocking-aviary1.0RC1+
Assignee: firefox → danm.moz
Priority: P3 → P2
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).
Keywords: top100
Flags: blocking-aviary1.0RC1+ → blocking-aviary1.0RC1-
mconnor, could you have a look at trying to come up for a fix for this?
Assignee: danm.moz → mconnor
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>
If you use space, would that cause a problem with "c:\program files" ?
> 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.
Assignee: mconnor → steffen.wilberg
QA Contact: mconnor
Attached patch patch (obsolete) — Splinter Review
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.
Attachment #155080 - Flags: review?(bugs)
Whiteboard: [have patch]
*** Bug 258598 has been marked as a duplicate of this bug. ***
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.

This bug still has not been fixed as of the new 1.0 release. What gives?
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 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.
Spaces would be a problem if someone is trying to load a local file and the path
has spaces in it, no?
Attached patch unbitrotted (obsolete) — Splinter Review
Attachment #155080 - Attachment is obsolete: true
Attachment #155080 - Flags: review?(bugs)
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
Attachment #160367 - Flags: review?(bugs)
Attachment #160367 - Attachment description: unbitrotten → unbitrotted
Attachment #160367 - Attachment is obsolete: true
Attachment #160367 - Flags: review?(bugs)
Attached patch unbitrotted, v2 (obsolete) — Splinter Review
Attachment #160368 - Flags: review?(bugs)
> (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?

> 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
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. 
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).
*** Bug 260969 has been marked as a duplicate of this bug. ***
Attachment #160368 - Attachment is obsolete: true
Attachment #160368 - Flags: review?(bugs)
Whiteboard: [have patch]
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.
Flags: blocking-aviary1.0+ → blocking-aviary1.0-
*** Bug 263413 has been marked as a duplicate of this bug. ***
*** Bug 264443 has been marked as a duplicate of this bug. ***
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.
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.
Sorry, I have very little time at the moment. -> mconnor per comment 40.
Assignee: steffen.wilberg → mconnor
QA Contact: mconnor → general
*** Bug 288154 has been marked as a duplicate of this bug. ***
Keywords: intl
Status: NEW → ASSIGNED
Target Milestone: --- → Firefox1.1
(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)? 
(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... 
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.
(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?
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.
Flags: blocking1.8b4?
*** Bug 301201 has been marked as a duplicate of this bug. ***
Flags: blocking-aviary1.1?
Flags: blocking1.8b4?
Flags: blocking1.8b4-
Flags: blocking-aviary1.5?
*** Bug 306047 has been marked as a duplicate of this bug. ***
*** Bug 310934 has been marked as a duplicate of this bug. ***
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.
*** Bug 313976 has been marked as a duplicate of this bug. ***
Flags: blocking-aviary2?
Flags: blocking-aviary2? → blocking-aviary2+
Flags: blocking1.8b5-
Flags: blocking-aviary1.0PR-
Flags: blocking-aviary1.0-
*** Bug 316158 has been marked as a duplicate of this bug. ***
Depends on: 318088
(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.
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.
Summary: Using "|" as URI separator breaks some links/home pages → Using "|" (pipe) as URI separator breaks some links/home pages; causes multiple tabs to open
Severity: normal → minor
Target Milestone: Firefox1.5 → Firefox 2 alpha2
Component: General → Tabbed Browser
QA Contact: general → tabbed.browser
Whiteboard: SWAG: 0 (to be fixed by 318088)
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 ????
(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.
bug 318088 moved with Places, but we need a fix for this in b1, this is a bugfix
Target Milestone: Firefox 2 alpha2 → Firefox 2 beta1
Whiteboard: SWAG: 0 (to be fixed by 318088) → SWAG: 3d
!?
this should depend on the prefwindow rewrite now
Target Milestone: Firefox 2 beta1 → Firefox 2 beta2
pushing out non-critical-path bugs to b2
Depends on: pref-reorg
Would a fix for this also affect bug 343999 ?
(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).
Blocks: 343999
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
This isn't going to make Fx2, unfortunately, but will be easily fixed for Places.
Flags: blocking-firefox2+ → blocking-firefox2-
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.
Duplicate of this bug: 378258
Flags: blocking-firefox3?
Target Milestone: Firefox 2 beta2 → Firefox 3 beta
Flags: blocking-firefox3? → blocking-firefox3+
Target Milestone: Firefox 3 M7 → Firefox 3 M8
Duplicate of this bug: 388747
Blocks: 248956
Duplicate of this bug: 388928
Target Milestone: Firefox 3 M8 → Firefox 3 M9
moving out bugs that don't need to block b1
Target Milestone: Firefox 3 M9 → Firefox 3 M10
how about new-line as the separator?
Target Milestone: Firefox 3 M10 → Firefox 3 M11
Priority: P2 → P5
Flags: wanted-firefox3+
Flags: blocking-firefox3-
Flags: blocking-firefox3+
Target Milestone: Firefox 3 Mx → ---
Indeed, like IE 8, we should have a multi-line field for that and put one URL on each line.  
Flags: wanted1.9.0.x?
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.
Assignee: mconnor → nobody
Status: ASSIGNED → NEW
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.
Flags: blocking-firefox3.1?
This isn't a blocker, maybe ever, but we should fix it at some point soon...
Flags: wanted-firefox3.1+
Flags: blocking-firefox3.1?
Flags: blocking-firefox3.1-
Priority: P5 → P3
Target Milestone: --- → Firefox 3.1
Flags: wanted1.9.0.x? → wanted1.9.0.x-
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)?
Depends on: CVE-2008-2933
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.
Assignee: nobody → gavin.sharp
Status: NEW → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
Target Milestone: Firefox 3.1 → Firefox 3.1a1
(We might still want to change the way we store multiple home pages in the future, but that can happen in a new bug.)
Whiteboard: SWAG: 3d
I reopened bug 244192 for that.
You need to log in before you can comment on or make changes to this bug.