Defect - Clicking links in Chatzilla/Skype (and others?) strips off the hash at the end of the URL

VERIFIED FIXED

Status

Tracking
Metro Operations
P1
normal
VERIFIED FIXED
6 years ago
4 years ago

People

(Reporter: KWierso, Assigned: jimm)

Tracking

x86_64
Windows 8
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox21- disabled)

Details

(Whiteboard: feature=defect c=content_features u=metro_firefox_user p=0 status=for_testing)

Attachments

(1 attachment, 1 obsolete attachment)

4.18 KB, patch
TimAbraldes
: review+
Details | Diff | Splinter Review
(Reporter)

Description

6 years ago
If there's a link posted in some channel that includes a hash at the end, the hash is not passed along to Firefox.  (So "https://bugzilla.mozilla.org/show_bug.cgi?id=793052#c2" will become "https://bugzilla.mozilla.org/show_bug.cgi?id=793052" when it is opened.)

Interestingly, if I right-click the link and choose to copy the link location, the copied link DOES include the hash.
(Reporter)

Comment 1

6 years ago
I'm using Chatzilla on XULRunner, if that makes a difference compared to the extension version.
Mozilla/5.0 (X11; Linux x86_64; rv:18.0) Gecko/18.0 Firefox/18.0 SeaMonkey/2.15a1 ID:20120921110141 c-c:f181aef1ee3f m-c:3c68fdd4f77a

ChatZilla 0.9.89

I do not see that bug in cZ running as an extension in SeaMonkey; but in my case the link is passed internally to the browser of the same app (i.e. the SeaMonkey browser, system default or not), while I suppose that "cZ in XR" would have to pass the link to "whatever is the default browser on this OS".

Wes: which Firefox version, on which OS? You may find it by pasting the line(s) labeled "User Agent" and/or "Build Identifier" at the bottom of the page which opens when you type "about:" (without the quotes, and followed by Enter) in the URL bar. It should be similar but not identical to what I pasted at the top of this comment.
P.S. Oh, and which version of ChatZilla? *That* is found at the top of the /client tab in cZ, or in the second column on the ChatZilla line of the "Help → Troubleshooting Information" page.
P.P.S. Ah, I don't know if XR has "Troubleshooting Information". Well, even cZ in XR has a client tab — just type /client to go to it.
(Reporter)

Comment 5

6 years ago
Build identifier: Mozilla/5.0 (Windows NT 6.2; WOW64; rv:18.0) Gecko/18.0 Firefox/18.0
ChatZilla 0.9.88-rdmsoft [XULRunner 1.9.0.17/2009122204]

This isn't a relatively recent issue for me, it's been happening at least since Nightly was in the 14.0a1 cycle, maybe earlier. It's been bugging me for a while, just never enough to actually get me to file the bug.

Might be a Windows 8 issue? I no longer have another OS to test this on, as I just deleted my Win7 partition yesterday.
What happens if you click a link with a fragment in an email (assuming that you read mail in a mailer, not by webmail in your browser)? Does the fragment also get sripped when your OS passes it to Firefox? Here is such a link, it should be in the bugmail Bugzilla sent you for this comment:
http://tools.ietf.org/html/rfc3986#section-3.5
Version: unspecified → Other Branch
(Reporter)

Comment 7

6 years ago
Doesn't work from Skype to Firefox, either.

Huh.
(Reporter)

Comment 8

6 years ago
But it does work from the Win8 Metro "Mail" app to the Metro version of Firefox...
(In reply to Wes Kocher (:KWierso) from comment #7)
> Doesn't work from Skype to Firefox, either.
> 
> Huh.

Hm. I suspect a problem with how Windows passes URLs to the default browser. However Microsoft is notoriously unhelpful toward customers who didn't pay through the nose to buy an official (non-OEM) copy of Windows plus professional support, so unless you are one of these paying customers, I don't know how you should proceed: When I was still on Windows, once I had a problem and sent them a support request by filling in a form on their site. The answer was: "Your version of Windows is an OEM version, meant to be sold only together with a computer. It is not supported by Microsoft but only by your computer vendor." (So what was I to do? The shop where I had bought the computer had gone bankrupt by then.)
(Reporter)

Comment 10

6 years ago
(In reply to Tony Mechelynck [:tonymec] from comment #9)
> Hm. I suspect a problem with how Windows passes URLs to the default browser.
> However Microsoft is notoriously unhelpful toward customers who didn't pay
> through the nose to buy an official (non-OEM) copy of Windows plus
> professional support, so unless you are one of these paying customers, I
> don't know how you should proceed: When I was still on Windows, once I had a
> problem and sent them a support request by filling in a form on their site.
> The answer was: "Your version of Windows is an OEM version, meant to be sold
> only together with a computer. It is not supported by Microsoft but only by
> your computer vendor." (So what was I to do? The shop where I had bought the
> computer had gone bankrupt by then.)

Well, I got my copy through my TechNet subscription, so I guess I am a direct paying customer. I'm gonna see if anyone else can reproduce this and track down if it is a Win8 regression before escalating this to MS, though.

Comment 11

6 years ago
(In reply to Wes Kocher (:KWierso) from comment #5)
> This isn't a relatively recent issue for me, it's been happening at least
> since Nightly was in the 14.0a1 cycle, maybe earlier. It's been bugging me
> for a while, just never enough to actually get me to file the bug.

Were you on Windows 8 the whole time it has been happening? This is unlikely to be a ChatZilla issue and obviously your XULRunner hasn't changed for some time (2009), which leave the OS and Firefox. My money's on Firefox. ;)
(Reporter)

Comment 12

6 years ago
(In reply to James Ross from comment #11)
> Were you on Windows 8 the whole time it has been happening? This is unlikely
> to be a ChatZilla issue and obviously your XULRunner hasn't changed for some
> time (2009), which leave the OS and Firefox. My money's on Firefox. ;)

I only remember actively noticing it while using the various test builds and the RTM build of Windows 8.
(Reporter)

Comment 13

6 years ago
(In reply to Wes Kocher (:KWierso) from comment #10)
> I'm gonna see if anyone else can reproduce this and
> track down if it is a Win8 regression before escalating this to MS, though.

Mossop says that it works correctly for him on Windows 7 using Chatzilla on XULRunner, so I guess there's something different in Windows 8 or Firefox.

Regardless, this happens from Skype > Firefox, so it's definitely not a Chatzilla bug.
Assignee: rginda → nobody
Component: ChatZilla → Shell Integration
Product: Other Applications → Firefox
Version: Other Branch → Trunk
(Reporter)

Updated

6 years ago
Summary: Clicking links in Chatzilla strips off the hash at the end of the URL → Clicking links in Chatzilla/Skype (and others?) strips off the hash at the end of the URL
fwiw I confirmed this bug on win8 on skype but not on win7 with skype.
(Reporter)

Comment 15

6 years ago
If desktop IE is the default browser, the hash is preserved from Skype to IE.
More findings:
If you type Start Menu Button + R, and then in the run dialog run:
http://www.bugzilla.mozilla.org/#home 
It'll strip off the anchor name.

If you run cmd.exe, cd into your installation directory and run:
firefox.exe -url http://www.bugzilla.mozilla.org/#home
It'll keep the anchor name.
We had a similar situation with the windows shell munging the command line. We used a standalone program to analyze all of the different permutations so we could do the right thing. I would suggest verifying that it is the shell that is stripping it and not something in our code by recording the command line sent to us early in the startup path just in case.

Updated

6 years ago
Keywords: qawanted
Whiteboard: [win8]
Assignee: nobody → netzen
(Reporter)

Comment 18

5 years ago
Actually, this only seems to happen for me on Windows 8 with the Firefox versions that include the Metro stuff. I was using Elm Nightlies when I reported this bug, and now that Elm has merged back into m-c, I see it with normal Nightlies.
Component: Shell Integration → Shell
Product: Firefox → Firefox for Metro
Version: Trunk → 21 Branch
Probably has to do with CEH which app invocations go through now only when MOZ_METRO is defined. Unassigned myself though due to prioritization.
Assignee: netzen → nobody

Updated

5 years ago
Blocks: 841214
(Assignee)

Updated

5 years ago
tracking-firefox21: --- → ?
I can reproduce this by clicking any link in Pidgin that includes a fragment identifier.

In a probably related issue, clicking HTTPS links in Pidgin is broken; it seems to pass Firefox a file: URI pointing to the Temporary Internet Files directory.
Assignee: nobody → mbrubeck
Keywords: qawanted
Whiteboard: [win8] → [win8][metro-mvp?]
When I click on "http://limpet.net/#test" I get this output from CommandExecuteHandler (built with with #define SHOW_CONSOLE):

SetSelection param count: 1
SetSelection param: 'http://limpet.net/'
SetSelection target: http://limpet.net/
Initialize(open)
IExecuteCommandApplicationHostEnvironment::GetValue()
Couldn't get IExecuteCommandHost service from explorer. (80004001)
Execute()
Desktop Launch: verb:open exe:c:\Users\mbrub_000\src\elm\obj-metro\dist\bin\firefox.exe params:-url
http://limpet.net/

and when I click any https URI in Pidgin, I get:

SetSelection param count: 1
SetSelection param: 'C:\Users\mbrub_000\AppData\Local\Microsoft\Windows\Temporary Internet Files\Content.IE5\DJCOP9UF\CA3S0V90'
SetSelection target: C:\Users\mbrub_000\AppData\Local\Microsoft\Windows\Temporary Internet Files\Content.IE5\DJCOP9UF\CA3S0V90
Initialize(open)
IExecuteCommandApplicationHostEnvironment::GetValue()
Couldn't get IExecuteCommandHost service from explorer. (80004001)
Execute()
Desktop Launch: verb:open exe:c:\Users\mbrub_000\src\elm\obj-metro\dist\bin\firefox.exe params:-url
C:\Users\mbrub_000\AppData\Local\Microsoft\Windows\Temporary Internet Files\Content.IE5\DJCOP9UF\CA3S0V90
Assignee: mbrubeck → jmathies
(Assignee)

Updated

5 years ago
Blocks: 841610
(Assignee)

Comment 22

5 years ago
> When I click on "http://limpet.net/#test" I get this output from
> CommandExecuteHandler (built with with #define SHOW_CONSOLE):
> 
> SetSelection param count: 1
> SetSelection param: 'http://limpet.net/'

I have this fixed. 


> and when I click any https URI in Pidgin, I get:
> 
> SetSelection param count: 1
> SetSelection param:
> 'C:\Users\mbrub_000\AppData\Local\Microsoft\Windows\Temporary Internet
> Files\Content.IE5\DJCOP9UF\CA3S0V90'

I'd like to create a try build and have you test this. not sure if it's fixed, as I don't know the exact path these take through the ceh.
(Assignee)

Comment 23

5 years ago
> > and when I click any https URI in Pidgin, I get:
> > 
> > SetSelection param count: 1
> > SetSelection param:
> > 'C:\Users\mbrub_000\AppData\Local\Microsoft\Windows\Temporary Internet
> > Files\Content.IE5\DJCOP9UF\CA3S0V90'
> 
> I'd like to create a try build and have you test this. not sure if it's
> fixed, as I don't know the exact path these take through the ceh.

https://tbpl.mozilla.org/?tree=Try&rev=a39dc68a6fa0

When you get a chance, can you take this build for a spin? The build has the ceh console hard coded on, with output that should help diagnose if anything goes wrong.
(Assignee)

Updated

5 years ago
Attachment #714298 - Attachment description: wup → wip
(Assignee)

Comment 25

5 years ago
Created attachment 714558 [details] [diff] [review]
patch
Attachment #714298 - Attachment is obsolete: true
Attachment #714558 - Flags: review?(tabraldes)
Considering metro is not riding the trains yet, this will not impact our Aurora Fx21 users.Hence no need to track for release.
tracking-firefox21: ? → -

Updated

5 years ago
status-firefox21: --- → disabled

Comment 27

5 years ago
This is a pretty serious defect in the fixed bug 831887. We should nail this down for sure.
Whiteboard: [win8][metro-mvp?] → feature=defect c=content_features u=metro_firefox_user

Updated

5 years ago
Blocks: 839392
No longer blocks: 841214
Priority: -- → P1
Summary: Clicking links in Chatzilla/Skype (and others?) strips off the hash at the end of the URL → Defect - Clicking links in Chatzilla/Skype (and others?) strips off the hash at the end of the URL
Whiteboard: feature=defect c=content_features u=metro_firefox_user → feature=defect c=content_features u=metro_firefox_user p=0
Comment on attachment 714558 [details] [diff] [review]
patch

Review of attachment 714558 [details] [diff] [review]:
-----------------------------------------------------------------

Looks good with a couple nits.  Hopefully we can find a way to fix/workaround the issues with "file:///" URLs that contain spaces, though I'm fine doing that as a followup (as long as we comment).  Is there a good way to test this functionality?

::: browser/metro/shell/commandexecutehandler/CommandExecuteHandler.cpp
@@ +438,5 @@
>    }
>    return false;
>  }
>  
> +FORMATETC* GetPlainTextFormat() {

Instead of functions that return statics, I'd like to see 

namespace {
  const FORMATETC kPlainTextFormat = {CF_TEXT, 0, DVASPECT_CONTENT, -1, TYMED_HGLOBAL};
  const FORMATETC kPlainTextWFormat = {CF_UNICODETEXT, 0, DVASPECT_CONTENT, -1, TYMED_HGLOBAL};
};

Then, anywhere we would have used GetPlainTextFormat or GetPlainTextWFormat, we use &kPlainTextFormat or &kPlainTextWFormat instead.

If it turns out that we want to use these values outside this file, we can move them into MetroUtils.cpp or a similar file.

@@ +463,5 @@
> +  STGMEDIUM store;
> +
> +  // unicode text
> +  if (SUCCEEDED(aDataObj->GetData(GetPlainTextWFormat(), &store))) {
> +    cstrText = static_cast<LPCWSTR>(GlobalLock(store.hGlobal));

It might be helpful to add a comment explaining that, since cstrText is a CString, the memory that store.hGlobal points to gets copied and can now be discarded.  This is just in case someone edits or copy+pastes this code in the future; we don't want them to accidentally use the memory pointed to by store.hGlobal after calling GlobalUnlock.

@@ +502,5 @@
> +    URL_COMPONENTS components = {0};
> +    components.lpszScheme = scheme;
> +    components.dwSchemeLength = sizeof(scheme)/sizeof(scheme[0]);
> +    components.dwStructSize = sizeof(components);
> +    if (!InternetCrackUrlW(cstrText, 0, 0, &components)) {

According to the MSDN documentation for InternetCrackUrl, it should not be used on "file://" URLs that contain spaces.  We should at the very least comment the fact that dwUrlPathLength will be wrong for "file://" URLs that contain spaces, if we can't find a workaround.
Attachment #714558 - Flags: review?(tabraldes) → review+

Updated

5 years ago
Blocks: 839143
No longer blocks: 839392
Whiteboard: feature=defect c=content_features u=metro_firefox_user p=0 → feature=defect c=content_features u=metro_firefox_user p=0 status=for_testing
(Assignee)

Comment 29

5 years ago
(In reply to Tim Abraldes (:tabraldes) from comment #28)
> @@ +502,5 @@
> > +    URL_COMPONENTS components = {0};
> > +    components.lpszScheme = scheme;
> > +    components.dwSchemeLength = sizeof(scheme)/sizeof(scheme[0]);
> > +    components.dwStructSize = sizeof(components);
> > +    if (!InternetCrackUrlW(cstrText, 0, 0, &components)) {
> 
> According to the MSDN documentation for InternetCrackUrl, it should not be
> used on "file://" URLs that contain spaces.  We should at the very least
> comment the fact that dwUrlPathLength will be wrong for "file://" URLs that
> contain spaces, if we can't find a workaround.

I'll add a comment. I don't think we'll have issues with the parsing here since we are only using this to detect the scheme.
(Assignee)

Comment 30

5 years ago
(In reply to Tim Abraldes (:tabraldes) from comment #28)
> Comment on attachment 714558 [details] [diff] [review]
> patch
> 
> Review of attachment 714558 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> Looks good with a couple nits.  Hopefully we can find a way to
> fix/workaround the issues with "file:///" URLs that contain spaces, though
> I'm fine doing that as a followup (as long as we comment).  Is there a good
> way to test this functionality?


Pretty much anything you launch on win8 goes through here. start -> run will trigger the code for various uri types.
(Assignee)

Updated

5 years ago
Blocks: 843020
(In reply to Jim Mathies [:jimm] from comment #30)
> (In reply to Tim Abraldes (:tabraldes) from comment #28)
> > Comment on attachment 714558 [details] [diff] [review]
> > patch
> > 
> > Review of attachment 714558 [details] [diff] [review]:
> > -----------------------------------------------------------------
> > 
> > Looks good with a couple nits.  Hopefully we can find a way to
> > fix/workaround the issues with "file:///" URLs that contain spaces, though
> > I'm fine doing that as a followup (as long as we comment).  Is there a good
> > way to test this functionality?
> 
> 
> Pretty much anything you launch on win8 goes through here. start -> run will
> trigger the code for various uri types.

Right, I was unclear: What I meant was, "would it be difficult to write some _automated_ tests for this?" :)
https://hg.mozilla.org/mozilla-central/rev/c64083c5696d
Status: NEW → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED

Comment 34

5 years ago
Verified with nightly built from from http://hg.mozilla.org/mozilla-central/rev/702d2814efbf  the hashes are passed in when clicking links with hashes in other apps .
Status: RESOLVED → VERIFIED

Updated

5 years ago
Blocks: 841099
No longer blocks: 839143

Updated

5 years ago
Component: Shell → Metro Operations
Product: Firefox for Metro → Tracking
Version: 21 Branch → ---
User Agent: Mozilla/5.0 (Windows NT 6.2; WOW64; rv:26.0) Gecko/20100101 Firefox/26.0
Build ID: 20130823030204
Built from http://hg.mozilla.org/mozilla-central/rev/fb2318875cd4

WFM
Tested on windows 8 using latest nightly for iteration-12. The hashes are passed in when clicking links with hashes in other apps .
You need to log in before you can comment on or make changes to this bug.