[Bug 25463] New: Regression: ZEMAX cannot display help pages

http://bugs.winehq.org/show_bug.cgi?id=25463

Summary: Regression: ZEMAX cannot display help pages
           Product: Wine
           Version: 1.3.6
          Platform: x86
        OS/Version: Linux
            Status: UNCONFIRMED
          Severity: normal
          Priority: P2
         Component: urlmon
        AssignedTo: [e-mail]
        ReportedBy: [e-mail]

ZEMAX can no-longer display help pages, reports the following errors:
---
fixme:shdocvw:BindStatusCallback_OnProgress status code 11
fixme:shdocvw:bind_to_object BindToObject failed: 80004005
---
and a bisect reveals:
---
6c1f2e4f3f96c71a90fcabc000a8d0ce4883bd29 is the first bad commit
commit 6c1f2e4f3f96c71a90fcabc000a8d0ce4883bd29
Author: Jacek Caban 
Date:   Wed Nov 10 13:23:34 2010 +0100

urlmon: Added IInternetProtocolEx support to BindProtocol.

:040000 040000 ac72ba621255c192e74732a60a49dd0f3603663e
7df787a144cfd951057cc0e61a690152948928c8 M    dlls
---

[Bug 25463] Regression: ZEMAX cannot display help pages

http://bugs.winehq.org/show_bug.cgi?id=25463

Erich Hoover  changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
           Keywords|                            |regression
                 CC|                            |[e-mail]

[Bug 25463] Regression: ZEMAX cannot display help pages

http://bugs.winehq.org/show_bug.cgi?id=25463

Dan Kegel  changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |[e-mail]

--- Comment #1 from Dan Kegel  2010-12-08 18:19:31 CST ---
Does 'winetricks urlmon' work around this, btw?

[Bug 25463] Regression: ZEMAX cannot display help pages

http://bugs.winehq.org/show_bug.cgi?id=25463

--- Comment #2 from Erich Hoover  2010-12-08 18:25:57 CST ---
(In reply to comment #1)
> Does 'winetricks urlmon' work around this, btw?

I don't think it'll be necessary to take that route.  Even though I'm not
familiar with this part of Wine, the relevant commit seems pretty
straightforward.  I'm still looking into the "real problem", but I can fix the
issue by replacing IUri_GetDisplayUri (only occurs once in bindprot.c) with
IUri_GetRawUri.

[Bug 25463] Regression: ZEMAX cannot display help pages

http://bugs.winehq.org/show_bug.cgi?id=25463

--- Comment #3 from Erich Hoover  2010-12-08 21:15:35 CST ---
Created an attachment (id=32417)
 --> (http://bugs.winehq.org/attachment.cgi?id=32417)
urlmon: Fix handling MK protocol URLs.

I believe the attached patch is the appropriate fix (unescaping the URL passed
to the MK handler).  However, since I'm not real familiar with this area of
Wine, any feedback would be greatly appreciated.

[Bug 25463] ZEMAX cannot display help pages

http://bugs.winehq.org/show_bug.cgi?id=25463

Dmitry Timoshkov  changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
            Summary|Regression: ZEMAX cannot    |ZEMAX cannot display help
                   |display help pages          |pages

[Bug 25463] ZEMAX cannot display help pages

http://bugs.winehq.org/show_bug.cgi?id=25463

--- Comment #4 from Jacek Caban  2010-12-09 15:35:29 CST ---
(In reply to comment #3)
> Created an attachment (id=32417)

--> (http://bugs.winehq.org/attachment.cgi?id=32417) [details]
> urlmon: Fix handling MK protocol URLs.
>
> I believe the attached patch is the appropriate fix (unescaping the URL passed
> to the MK handler).  However, since I'm not real familiar with this area of
> Wine, any feedback would be greatly appreciated.

It requires some tests. I have some IInternetProtocolEx tests that may help
here pending to fix random failures and send. I hope to do so next week.

(In reply to comment #1)
> Does 'winetricks urlmon' work around this, btw?

Please stop suggesting it. It can't work and will break mshtml. See
http://code.google.com/p/winezeug/issues/detail?id=201

[Bug 25463] ZEMAX cannot display help pages

http://bugs.winehq.org/show_bug.cgi?id=25463

--- Comment #5 from Dan Kegel  2010-12-09 15:53:22 CST ---
Mostly I was looking for evidence that I could delete that verb :-)

[Bug 25463] ZEMAX cannot display help pages

http://bugs.winehq.org/show_bug.cgi?id=25463

--- Comment #6 from Jacek Caban  2010-12-09 16:04:25 CST ---
(In reply to comment #5)
> Mostly I was looking for evidence that I could delete that verb :-)

We call CreateUri here:
http://source.winehq.org/git/wine.git/?a=blob;f=dlls/mshtml/nsio.c;h=aef54ee...
and it's available since IE7:
http://msdn.microsoft.com/en-us/library/ms775098%28VS.85%29.aspx
so it's not available in urlmon.dll installed by winetricks.

[Bug 25463] ZEMAX cannot display help pages

http://bugs.winehq.org/show_bug.cgi?id=25463

--- Comment #7 from Dan Kegel  2010-12-11 18:37:42 CST ---
but lo, bug 25492 is helped by the broken winetricks urlmon, so
maybe it can hang around a bit...

[Bug 25463] ZEMAX cannot display help pages

http://bugs.winehq.org/show_bug.cgi?id=25463

--- Comment #8 from Jacek Caban  2010-12-12 10:04:36 CST ---
After using |winetricks urlmon|, running simple |wine iexplore| crashes. That's
not a setup we want to encourage in any way. A slight improvement in bug 25492
is not worth it. It should be fixed, not worked around by breaking the
configuration.

Let's move the discussion off to winetricks' issues tracker.

[Bug 25463] ZEMAX cannot display help pages

http://bugs.winehq.org/show_bug.cgi?id=25463

--- Comment #9 from Erich Hoover  2010-12-21 09:58:58 CST ---
(In reply to comment #4)
> (In reply to comment #3)
> > Created an attachment (id=32417)

--> (http://bugs.winehq.org/attachment.cgi?id=32417) [details] [details]
> > urlmon: Fix handling MK protocol URLs.
> >
> > I believe the attached patch is the appropriate fix (unescaping the URL passed
> > to the MK handler).  However, since I'm not real familiar with this area of
> > Wine, any feedback would be greatly appreciated.
>
> It requires some tests. I have some IInternetProtocolEx tests that may help
> here pending to fix random failures and send. I hope to do so next week.

Would you mind providing a pre-release of these tests?  I know I'll still need
to wait for the finished product, but I'll be on vacation soon and will have
more spare time to dedicate to working on this task.

[Bug 25463] ZEMAX cannot display help pages

http://bugs.winehq.org/show_bug.cgi?id=25463

--- Comment #10 from Jacek Caban  2010-12-21 10:06:30 CST ---
Sorry about the delay, I couldn't find the time to work on this. You can find
them here:
http://www.winehq.org/pipermail/wine-patches/2010-November/095247.html
They didn't get into the tree because of failures on winetestbot.

[Bug 25463] ZEMAX cannot display help pages

http://bugs.winehq.org/show_bug.cgi?id=25463

--- Comment #11 from Erich Hoover  2010-12-21 10:13:33 CST ---
(In reply to comment #10)
> Sorry about the delay, I couldn't find the time to work on this. You can find
> them here:
> http://www.winehq.org/pipermail/wine-patches/2010-November/095247.html
> They didn't get into the tree because of failures on winetestbot.

No problem, you do look to be pretty busy from your high commit rate lately. 
Thanks so much!

[Bug 25463] ZEMAX cannot display help pages

http://bugs.winehq.org/show_bug.cgi?id=25463

--- Comment #12 from Jacek Caban  2010-12-23 11:22:17 CST ---
These tests are in Git now. The things to look for are:

- are URL->IUri and IUri->URL done correctly (that's straightforward with these
tests)
- if yes, how should mk: protocol be fixed (probably by implementing StartEx
method)

Possibly both part should be fixed. The first seems more critical since it
affects all bindings.

[Bug 25463] ZEMAX cannot display help pages

http://bugs.winehq.org/show_bug.cgi?id=25463

--- Comment #13 from Erich Hoover  2011-01-03 13:45:27 CST ---
(In reply to comment #12)
> These tests are in Git now. The things to look for are:
> - are URL->IUri and IUri->URL done correctly (that's straightforward with these
>
> tests)
> - if yes, how should mk: protocol be fixed (probably by implementing StartEx
>
> method)
>
> Possibly both part should be fixed. The first seems more critical since it
> affects all bindings.

I haven't had as much time to look into this as I'd like, but isn't that set of
cases already covered by the tests in uri.c?  In particular, there's a test
that shows that spaces get converted into web-form for AutoCAD:
create/raw form:
mk:@MSITStore:C:\Program Files/AutoCAD
2008\Help/acad_acg.chm::/WSfacf1429558a55de1a7524c1004e616f8b-322b.htm
display form:
mk:@MSITStore:C:\Program%20Files/AutoCAD%202008\Help/acad_acg.chm::/WSfacf1429558a55de1a7524c1004e616f8b-322b.htm

It seems to me that in order to test any further there'd need to be an ITStore
included in the tests.

[Bug 25463] ZEMAX cannot display help pages

http://bugs.winehq.org/show_bug.cgi?id=25463

--- Comment #14 from Jacek Caban  2011-01-05 08:45:37 CST ---
(In reply to comment #13)
> (In reply to comment #12)
> > These tests are in Git now. The things to look for are:
> > - are URL->IUri and IUri->URL done correctly (that's straightforward with these
> >
> > tests)
> > - if yes, how should mk: protocol be fixed (probably by implementing StartEx
> >
> > method)
> >
> > Possibly both part should be fixed. The first seems more critical since it
> > affects all bindings.
>
> I haven't had as much time to look into this as I'd like, but isn't that set of
> cases already covered by the tests in uri.c?

Not really. IUri may behave differently depending on flags used, we want to
check how BindProtocol is supposed to use it, not how IUri works. Anyway, I've
verified that we do things right (at least in case of URL you've posted, no
patch with tests yet), so the problem is in mk: protocol handler (we should
probably fix it by implementing its IInternetProtocolEx iface).

[Bug 25463] ZEMAX cannot display help pages

http://bugs.winehq.org/show_bug.cgi?id=25463

Jacek Caban  changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
             Blocks|                            |23622

[Bug 25463] ZEMAX cannot display help pages

http://bugs.winehq.org/show_bug.cgi?id=25463

Erich Hoover  changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
  Attachment #32417|0                           |1
        is obsolete|                            |

--- Comment #15 from Erich Hoover  2011-01-09 14:56:24 CST ---
Created an attachment (id=32791)
 --> (http://bugs.winehq.org/attachment.cgi?id=32791)
Update MK protocol handling to IInternetProtocolEx

Is this a bit more like what you're looking for?

[Bug 25463] ZEMAX cannot display help pages

http://bugs.winehq.org/show_bug.cgi?id=25463

Erich Hoover  changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
  Attachment #32791|0                           |1
        is obsolete|                            |

--- Comment #16 from Erich Hoover  2011-01-10 11:49:19 CST ---
Created an attachment (id=32797)
 --> (http://bugs.winehq.org/attachment.cgi?id=32797)
Update MK protocol handling to IInternetProtocolEx [v2]

Of course someone changed the file almost immediately (attached updated version
for new git).

[Bug 25463] ZEMAX cannot display help pages

http://bugs.winehq.org/show_bug.cgi?id=25463

--- Comment #17 from Jacek Caban  2011-01-10 15:51:58 CST ---
Thanks for the patch. I really hope we can avoid old-style UrlUnescape call
here. I was expecting IUri APIs to allow us that, but I did some tests and it
seems like it won't help here. Perhaps itss is responsible for unescaping?

Given that UrlUnescape is not related to the rest of the patch, these should be
two separated patches. Sorry for giving you wrong pointer about
IInternetProtocolEx, it won't help for this particular bug at all, but since
you've already written it, let's get it to the tree. Here are a few comments:

-static HRESULT WINAPI MkProtocol_Start(IInternetProtocol *iface, LPCWSTR
szUrl,
+static HRESULT WINAPI MkProtocol_StartEx(IInternetProtocolEx *iface, IUri
*pUri,

Please keep the order of function implementation matching vtbl layout. It means
that StartEx should be the last implemented function. If you want to reduce
patch size, you can duplicate Start code (with changes for using IUri) inside
the new StartEx function and forward Start to StartEx in a separated patch.

+    BSTR szUrl, szPathTmp, szPath = NULL;

Please don't use Hungarian notation. url, path and path_tmp are would be
preferable names.

+    hres = IUri_GetDisplayUri(pUri, &szUrl);
+    if(FAILED(hres))
+        return hres;
+    hres = IUri_GetPath(pUri, &szPathTmp);
+    if(FAILED(hres))
+        goto done;

If you moved these calls to the place there you need their results and free
resources as soon as they are no longer needed, you will avoid some gotos and
holding useless data.

progid = heap_alloc((ptr-ptr2+1)*sizeof(WCHAR));
     memcpy(progid, ptr2, (ptr-ptr2)*sizeof(WCHAR));

Since you own the memory of path here, you can avoid this allocation.

+    szPath = heap_alloc(path_size);
+    hres = UrlUnescapeW((LPWSTR)szPathTmp, szPath, &path_size, 0);
+    SysFreeString(szPathTmp);

Let's leave unescaping for later patch.

[Bug 25463] ZEMAX cannot display help pages

http://bugs.winehq.org/show_bug.cgi?id=25463

Erich Hoover  changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
  Attachment #32797|0                           |1
        is obsolete|                            |

--- Comment #18 from Erich Hoover  2011-01-10 19:46:17 CST ---
Created an attachment (id=32809)
 --> (http://bugs.winehq.org/attachment.cgi?id=32809)
Update MK protocol handling to IInternetProtocolEx [v3]

(In reply to comment #17)
> Thanks for the patch. I really hope we can avoid old-style UrlUnescape call
> here. I was expecting IUri APIs to allow us that, but I did some tests and it
> seems like it won't help here. Perhaps itss is responsible for unescaping?
> ...

I don't believe so, I through together a hacky test when I first started
looking into this issue. To do that I hand-coding a call to open a valid CHM
(with spaces in the filename) with IParseDisplayName_ParseDisplayName.  I'm not
sure if there's a good way to make a real test for this, but when I tested that
hack I did not get a moniker back.

Also, attached is a patch for updating just IInternetProtocolEx, please see if
this sufficiently addresses your concerns.

[Bug 25463] ZEMAX cannot display help pages

http://bugs.winehq.org/show_bug.cgi?id=25463

--- Comment #19 from Jacek Caban  2011-01-11 12:11:54 CST ---
(In reply to comment #18)
> Created an attachment (id=32809)

--> (http://bugs.winehq.org/attachment.cgi?id=32809) [details]
> Update MK protocol handling to IInternetProtocolEx [v3]
>
> (In reply to comment #17)
> > Thanks for the patch. I really hope we can avoid old-style UrlUnescape call
> > here. I was expecting IUri APIs to allow us that, but I did some tests and it
> > seems like it won't help here. Perhaps itss is responsible for unescaping?
> > ...
>
> I don't believe so, I through together a hacky test when I first started
> looking into this issue. To do that I hand-coding a call to open a valid CHM
> (with spaces in the filename) with IParseDisplayName_ParseDisplayName.

Good to know you already did that. Sadly it means that we really need
unescaping in mk protocol handler.
> I'm not sure if there's a good way to make a real test for this, but when I tested that
> hack I did not get a moniker back.

We could just escape URL in any way.
> Also, attached is a patch for updating just IInternetProtocolEx, please see if
> this sufficiently addresses your concerns.

It looks good now. Please send the patch to wine-patches.

[Bug 25463] ZEMAX cannot display help pages

http://bugs.winehq.org/show_bug.cgi?id=25463

Erich Hoover  changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|UNCONFIRMED                 |RESOLVED
         Resolution|                            |FIXED

--- Comment #20 from Erich Hoover  2011-01-14 14:02:08 CST ---
Fixed by commit aaa9fa711260da7272dc7d2330e6c366fac04904.

[Bug 25463] ZEMAX cannot display help pages

http://bugs.winehq.org/show_bug.cgi?id=25463

Alexandre Julliard  changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|RESOLVED                    |CLOSED

--- Comment #21 from Alexandre Julliard  2011-01-21 13:43:25 CST ---
Closing bugs fixed in 1.3.12.

[Bug 25463] ZEMAX cannot display help pages

https://bugs.winehq.org/show_bug.cgi?id=25463

Anastasius Focht  changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
      Fixed by SHA1|                            |aaa9fa711260da7272dc7d2330e
                   |                            |6c366fac04904
                 CC|                            |[e-mail]
    Regression SHA1|                            |6c1f2e4f3f96c71a90fcabc000a
                   |                            |8d0ce4883bd29