Re: [PATCH 1/2] mshtml: Reimplement IHTMLLocation::get_href

[Jacek Caban]

Hi Andrew,

Andrew Eikum wrote:
> ---
>  dlls/mshtml/htmllocation.c       |   93 
> +++++++++++++++++++++++++++++++++++--
>  dlls/mshtml/tests/htmllocation.c |    4 +-
>  2 files changed, 90 insertions(+), 7 deletions(-)

What's IHTMLDocument2::URL value in these cases? I'd expect it to be the 
same, which proves that get_hres is not the right place to reformat URL. 
Also I think that what you can see is a result of using IUri interface 
for storing URL. Then the right way of fixing it would be to use it in 
our implementation (it was introduced in IE7 and our loading code is 
mostly compatible with IE6 ATM). We could add your code as a temporary 
hack if it would be needed for a real application. Is it a case?


Thanks,
    Jacek

[PATCH 1/2] mshtml: Reimplement IHTMLLocation::get_href

[Andrew Eikum]

Jacek Caban wrote:
> Hi Andrew,
>
> Andrew Eikum wrote:
> > ---
> >  dlls/mshtml/htmllocation.c       |   93 
> > +++++++++++++++++++++++++++++++++++--
> >  dlls/mshtml/tests/htmllocation.c |    4 +-
> >  2 files changed, 90 insertions(+), 7 deletions(-)
>
> What's IHTMLDocument2::URL value in these cases? I'd expect it to be the 
> same, which proves that get_hres is not the right place to reformat URL.

No, they're not quite the same.  Using IE8, IHTMLDocument2::get_URL gives:

L"file://C:\\windows\\win.ini"

while IHTMLLocation::get_href gives:

L"file:///C:/windows/win.ini"

as shown in the tests.
> Also I think that what you can see is a result of using IUri interface 
> for storing URL. Then the right way of fixing it would be to use it in 
> our implementation (it was introduced in IE7 and our loading code is 
> mostly compatible with IE6 ATM). We could add your code as a temporary 
> hack if it would be needed for a real application. Is it a case?

I think it's likely that IHTMLLocation does use IUri.  MSDN states that 
IUri normalizes the URI, which would cause the change shown above.

Would you prefer some experimentation with IUri to see if they return 
expected values, then implement IUri first and IHTMLLocation after?  Or 
continue with what I've got and move the logic to IUri later if that 
turns out to be the case?

Andrew

[PATCH 1/2] mshtml: Reimplement IHTMLLocation::get_href

[Jacek Caban]

Andrew Eikum wrote:
> Jacek Caban wrote:
> > Hi Andrew,
> >
> > Andrew Eikum wrote:
> > > ---
> > >  dlls/mshtml/htmllocation.c       |   93 
> > > +++++++++++++++++++++++++++++++++++--
> > >  dlls/mshtml/tests/htmllocation.c |    4 +-
> > >  2 files changed, 90 insertions(+), 7 deletions(-)
> >
> > What's IHTMLDocument2::URL value in these cases? I'd expect it to be 
> > the same, which proves that get_hres is not the right place to 
> > reformat URL.
>
> No, they're not quite the same.  Using IE8, IHTMLDocument2::get_URL 
> gives:
>
> L"file://C:\\windows\\win.ini"
>
> while IHTMLLocation::get_href gives:
>
> L"file:///C:/windows/win.ini"
>
> as shown in the tests.

Thanks for testing it, then get_href is the right place for such hacks.
> > Also I think that what you can see is a result of using IUri 
> > interface for storing URL. Then the right way of fixing it would be 
> > to use it in our implementation (it was introduced in IE7 and our 
> > loading code is mostly compatible with IE6 ATM). We could add your 
> > code as a temporary hack if it would be needed for a real 
> > application. Is it a case?
>
> I think it's likely that IHTMLLocation does use IUri.  MSDN states 
> that IUri normalizes the URI, which would cause the change shown above.
>
> Would you prefer some experimentation with IUri to see if they return 
> expected values, then implement IUri first and IHTMLLocation after?  
> Or continue with what I've got and move the logic to IUri later if 
> that turns out to be the case?

Let's use your solution for now. Using IUri involves much more changes 
than just IHTMLLocation and your solution looks acceptable to me.


Thanks,
    Jacek