1436589 - Use nsIFileURLMutator::SetFile in nsWebBrowserPersist.cpp

Status: RESOLVED FIXED

(Core :: Networking, enhancement, P3)

Description

[Valentin Gosu [:valentin] (he/him)]

It seems merely changing the code to use a mutator breaks web page saving.
This requires a unit test too.

Comment 1

[Valentin Gosu [:valentin] (he/him)]

Attachment: Bug 1436589 - Use nsIFileURLMutator::SetFile in nsWebBrowserPersist.cpp

Review commit: https://reviewboard.mozilla.org/r/223054/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/223054/

Comment 2

[Andrea Marchesini [:baku]]

Comment on attachment 8953904 [details]
Bug 1436589 - Use nsIFileURLMutator::SetFile in nsWebBrowserPersist.cpp

https://reviewboard.mozilla.org/r/223054/#review229016

::: dom/webbrowserpersist/nsWebBrowserPersist.cpp:877
(Diff revision 1)
>                  data->mFile = uniqueFilenameURI;
>              }
> +
> +            // The URIData entry is pointing to the old unfixed URI, so we need
> +            // to update it.
> +            nsAutoCString spec;

move this near GetSpec()

Comment 3

[Honza Bambas (:mayhemer)]

Comment on attachment 8953904 [details]
Bug 1436589 - Use nsIFileURLMutator::SetFile in nsWebBrowserPersist.cpp

https://reviewboard.mozilla.org/r/223054/#review229112

::: dom/webbrowserpersist/nsWebBrowserPersist.cpp:862
(Diff revision 1)
>  
>          if (data->mCalcFileExt && !(mPersistFlags & PERSIST_FLAGS_DONT_CHANGE_FILENAMES))
>          {
>              nsCOMPtr<nsIURI> uriWithExt;
>              // this is the first point at which the server can tell us the mimetype
> +

X

::: dom/webbrowserpersist/nsWebBrowserPersist.cpp:2136
(Diff revision 1)
> -            nsresult rv;
> -            nsCOMPtr<nsIFileURL> fileURL = do_QueryInterface(aURI, &rv);
> -            NS_ENSURE_SUCCESS(rv, NS_ERROR_FAILURE);
> -            fileURL->SetFile(localFile);  // this should recalculate uri
> +            return NS_MutateURI(aURI)
> +                     .Apply(NS_MutatorMethod(&nsIFileURLMutator::SetFile,
> +                                             localFile))
> +                     .Finalize(aOutURI);
>          }
>          else

else after return

::: dom/webbrowserpersist/nsWebBrowserPersist.cpp:2314
(Diff revision 1)
> -                    nsCOMPtr<nsIFileURL> fileURL = do_QueryInterface(aURI, &rv);
> -                    NS_ENSURE_SUCCESS(rv, NS_ERROR_FAILURE);
> -                    fileURL->SetFile(localFile);  // this should recalculate uri
> +                    return NS_MutateURI(url)
> +                             .Apply(NS_MutatorMethod(&nsIFileURLMutator::SetFile,
> +                                                     localFile))
> +                             .Finalize(aOutURI);
>                  }
>                  else

else after return

Comment 4

[Valentin Gosu [:valentin] (he/him)]

Comment on attachment 8953904 [details]
Bug 1436589 - Use nsIFileURLMutator::SetFile in nsWebBrowserPersist.cpp

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/223054/diff/1-2/

Comment 5

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/2fa454395a88
Use nsIFileURLMutator::SetFile in nsWebBrowserPersist.cpp r=baku,mayhemer

Comment 6

[Arthur Iakab [arthur_iakab]]

https://hg.mozilla.org/mozilla-central/rev/2fa454395a88