Closed Bug 430910 Opened 15 years ago Closed 12 years ago

Rich text editor: pasted links have wrong number of leading "../" if editor page query string contains "/"

Categories

(Core :: DOM: Editor, defect, P2)

x86
Windows XP
defect

Tracking

()

RESOLVED FIXED
mozilla2.0b7
Tracking Status
blocking2.0 --- -
status2.0 --- wanted

People

(Reporter: cacyclewp, Assigned: ehsan.akhgari)

References

()

Details

Attachments

(2 files, 3 obsolete files)

User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8.1.12) Gecko/20080201 Firefox/2.0.0.12
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8.1.14) Gecko/20080404 Firefox/2.0.0.14

Links pasted into an iframe in designmode have the wrong number of leading "../" if page of the iframe page contains "/" as part of the query string.

I guess that the query string should theoretically not contain a "/" because it is supposed to be URLencoded. However, major websites including Wikipedia make use of such links. It should be relatively easy to fix this by stopping to parse the URL at the first "?". 

This bug currently breaks the HTML-to-wikicode conversion of the Wikipedia editor wikEd.

Reproducible: Always

Steps to Reproduce:
1. Go to http://www.kevinroth.com/rte/demo.htm?test=a/b/c/d/e/f/g/h
2. Paste the link above the edit window into the edit area
3. Check the "Show source" box 
Actual Results:  
Page URL: http://www.kevinroth.com/rte/demo.htm?test=a/b/c/d/e/f/g/h
Link URL: http://www.kevinroth.com/rte/
Pasted HTML: <a href="../../../../../../../">Cross-Browser Rich Text Editor (RTE) home page</a>

Expected Results:  
Correct HTML: <a href="./">Cross-Browser Rich Text Editor (RTE) home page</a>
Still broken in Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.1.1) Gecko/20090715 Firefox/3.5.1 (.NET CLR 3.5.30729). This bug still affects web applications, including the Wikipedia editor wikEd and it is not possible to create a workaround. Please also note the high number of votes. This might be a very simple bug to fix.
Still broken in 3.7a1pre. Please, can somebody fix this. It looks like a really easy to fix bug and it breaks a major feature of the Wikipedia editor wikEd (since almost two years!).

(Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.3a1pre) Gecko/20090927 Minefield/3.7a1pre (.NET CLR 3.5.30729))
Severity: normal → major
Priority: -- → P2
Version: unspecified → Trunk
Component: General → Editor
Product: Firefox → Core
QA Contact: general → editor
I was thinking this would be a case of editor doing its own URL mangling.  However, it's not; this is a bug in nsStandardURL::GetRelativeSpec:

(gdb) p *this
$44 = (nsStandardURL) {<nsIFileURL> = {<nsIURL> = {<nsIURI> = {<nsISupports> = {_vptr.nsISupports = 0x7f4b07440330}, <No data fields>}, <No data fields>}, <No data fields>}, <nsIStandardURL> = {<nsIMutable> = {<nsISupports> = {
        _vptr.nsISupports = 0x7f4b07440550}, <No data fields>}, <No data fields>}, <nsISerializable> = {<nsISupports> = {
      _vptr.nsISupports = 0x7f4b07440590}, <No data fields>}, <nsIClassInfo> = {<nsISupports> = {_vptr.nsISupports = 0x7f4b074405c8}, <No data fields>}, 
  mRefCnt = {mValue = 13}, _mOwningThread = {mThread = 0x144aab0}, 
  mSpec = {<nsACString_internal> = {
      mData = 0x43c2718 "http://www.kevinroth.com/rte/demo.htm?test=a/b/c/d/e/f/g/h", mLength = 58, mFlags = 5}, <No data fields>}, mDefaultPort = 80, 
  mPort = -1, mScheme = {mPos = 0, mLen = 4}, mAuthority = {mPos = 7, 
    mLen = 17}, mUsername = {mPos = 7, mLen = -1}, mPassword = {mPos = 7, 
    mLen = -1}, mHost = {mPos = 7, mLen = 17}, mPath = {mPos = 24, mLen = 34}, 
  mFilepath = {mPos = 24, mLen = 13}, mDirectory = {mPos = 24, mLen = 5}, 
  mBasename = {mPos = 29, mLen = 4}, mExtension = {mPos = 34, mLen = 3}, 
  mParam = {mPos = 24, mLen = -1}, mQuery = {mPos = 38, mLen = 20}, mRef = {
    mPos = 24, mLen = -1}, mOriginCharset = {<nsACString_internal> = {
      mData = 0x7f4b16f1cb68 "", mLength = 0, mFlags = 1}, <No data fields>}, 
  mParser = {mRawPtr = 0x1662270}, mFile = {mRawPtr = 0x0}, mHostA = 0x0, 
  mHostEncoding = 1, mSpecEncoding = 1, mURLType = 2, mMutable = 0, 
  mSupportsFileURL = 0, static gIDN = 0x1632100, static gCharsetMgr = 0x0, 
  static gInitialized = 1, static gEscapeUTF8 = 1, 
  static gAlwaysEncodeInUTF8 = 1, static gEncodeQueryInUTF8 = 0, 
  mDebugCList = {next = 0x2e1dd90, prev = 0x2c5a610}}
(gdb) p *uri2
$45 = (nsStandardURL) {<nsIFileURL> = {<nsIURL> = {<nsIURI> = {<nsISupports> = {_vptr.nsISupports = 0x7f4b07440330}, <No data fields>}, <No data fields>}, <No data fields>}, <nsIStandardURL> = {<nsIMutable> = {<nsISupports> = {
        _vptr.nsISupports = 0x7f4b07440550}, <No data fields>}, <No data fields>}, <nsISerializable> = {<nsISupports> = {
      _vptr.nsISupports = 0x7f4b07440590}, <No data fields>}, <nsIClassInfo> = {<nsISupports> = {_vptr.nsISupports = 0x7f4b074405c8}, <No data fields>}, 
  mRefCnt = {mValue = 1}, _mOwningThread = {mThread = 0x144aab0}, 
  mSpec = {<nsACString_internal> = {
      mData = 0x274dc78 "http://www.kevinroth.com/rte/", mLength = 29, 
      mFlags = 5}, <No data fields>}, mDefaultPort = 80, mPort = -1, 
  mScheme = {mPos = 0, mLen = 4}, mAuthority = {mPos = 7, mLen = 17}, 
  mUsername = {mPos = 7, mLen = -1}, mPassword = {mPos = 7, mLen = -1}, 
  mHost = {mPos = 7, mLen = 17}, mPath = {mPos = 24, mLen = 5}, mFilepath = {
    mPos = 24, mLen = 5}, mDirectory = {mPos = 24, mLen = 5}, mBasename = {
    mPos = 29, mLen = 0}, mExtension = {mPos = 29, mLen = -1}, mParam = {
    mPos = 24, mLen = -1}, mQuery = {mPos = 24, mLen = -1}, mRef = {mPos = 24, 
    mLen = -1}, mOriginCharset = {<nsACString_internal> = {
      mData = 0x7f4b16f1cb68 "", mLength = 0, mFlags = 1}, <No data fields>}, 
  mParser = {mRawPtr = 0x1662270}, mFile = {mRawPtr = 0x0}, mHostA = 0x0, 
  mHostEncoding = 1, mSpecEncoding = 0, mURLType = 2, mMutable = 1, 
  mSupportsFileURL = 0, static gIDN = 0x1632100, static gCharsetMgr = 0x0, 
  static gInitialized = 1, static gEscapeUTF8 = 1, 
  static gAlwaysEncodeInUTF8 = 1, static gEncodeQueryInUTF8 = 0, 
  mDebugCList = {next = 0x7f4b0744c680, prev = 0x3efe510}}
(gdb) p aResult
$46 = (nsACString_internal &) @0x7fff2438c860: {
  mData = 0x7fff2438c880 "../../../../../../../", mLength = 21, mFlags = 65553}
Status: UNCONFIRMED → NEW
Ever confirmed: true
1981     // need to account for slashes and add corresponding "../"
1982     while (*thisIndex)
1983     {
1984         if (*thisIndex == '/')
1985             aResult.AppendLiteral("../");
1986 
1987         thisIndex++;
1988     }

We should not be advancing past mSpec.get() + mParam.mPos or so (some of those positions might be -1, so have to be a little careful).
This bug still affects Gecko-20100818(Firefox 4.0b4). In Safari 5.01 the pasted url is <a href="http://www.kevinroth.com/rte/">Cross-Browser Rich Text Editor (RTE) home page</a>.
There's no attempt to convert the url to relative. I would like to look into resolving this.
Assignee: nobody → ehsan
I think I have a patch for Gecko/20100902 - Minefield/4.0b4. Should I upload it.
(In reply to comment #6)
> I think I have a patch for Gecko/20100902 - Minefield/4.0b4. Should I upload
> it.

Please!
This patch was made against the 2.0b4 source. This should fix the problem described here. I also noticed this comment.
// todo:  also check for file matches with '#', '?' and ';'
Should this be a separate issue?
> Should this be a separate issue?
Never-mind I'm confusing myself.
Attached patch revised b5 patch (obsolete) — Splinter Review
Attachment #471726 - Attachment is obsolete: true
Attachment #475864 - Flags: review?(michaelkohler)
still b4 source for compare.
Comment on attachment 475864 [details] [diff] [review]
revised b5 patch

I have no idea about this bug/code/module and I'm not a peer anyway -> canceling review.
Attachment #475864 - Flags: review?(michaelkohler) → review?
Attached patch Patch (v1) (obsolete) — Splinter Review
I think this is the correct fix here...
Attachment #475864 - Attachment is obsolete: true
Attachment #478340 - Flags: review?(bzbarsky)
Attachment #478340 - Flags: approval2.0?
Attachment #475864 - Flags: review?
Comment on attachment 478340 [details] [diff] [review]
Patch (v1)

Looks much better.
Comment on attachment 478340 [details] [diff] [review]
Patch (v1)

Was there a reason that:

    const char *limit = mSpec.get() + mFilePath.mPos + mFilePath.mLen;

(without any subtraction) didn't work?
Attached patch Patch (v2)Splinter Review
(In reply to comment #15)
> Comment on attachment 478340 [details] [diff] [review]
> Patch (v1)
> 
> Was there a reason that:
> 
>     const char *limit = mSpec.get() + mFilePath.mPos + mFilePath.mLen;
> 
> (without any subtraction) didn't work?

I must have missed it somehow, but it definitely does work.
Attachment #478340 - Attachment is obsolete: true
Attachment #478838 - Flags: review?(bzbarsky)
Attachment #478838 - Flags: approval2.0?
Attachment #478340 - Flags: review?(bzbarsky)
Attachment #478340 - Flags: approval2.0?
Comment on attachment 478838 [details] [diff] [review]
Patch (v2)

r=me

I'd like someone else to look at the approval request, though... I do think this is safe enough.
Attachment #478838 - Flags: review?(bzbarsky) → review+
Whiteboard: [needs landing]
http://hg.mozilla.org/mozilla-central/rev/4f09e912977d
Status: NEW → RESOLVED
Closed: 12 years ago
Flags: in-testsuite+
Resolution: --- → FIXED
Whiteboard: [needs landing]
Target Milestone: --- → mozilla2.0b8
Target Milestone: mozilla2.0b8 → mozilla2.0b7
You need to log in before you can comment on or make changes to this bug.