Closed Bug 1179967 Opened 6 years ago Closed 6 years ago

Save As (nsWebBrowserPersist) shouldn't modify the document

Categories

(Firefox :: General, defect)

defect
Not set
normal

Tracking

()

RESOLVED FIXED
Firefox 42
Tracking Status
firefox42 --- fixed

People

(Reporter: jld, Assigned: jld)

References

Details

Attachments

(1 file)

“Save As” with the “Text File” option (currently non-e10s-only), or more generally nsIWebBrowserPersist::SaveDocument with a null data path, will side-effect the document to add a <base> element rather than use the code it already has to convert links to absolute.  This is a strange behavior, and it would be nice if I didn't have to maintain it in bug 1101100 (and I'd also like to keep that bug as close to “no functional change intended” as feasible).
Looking at nsWebBrowserPersist::SetDocumentBase(), it can also create a <head>. Blame shows that this is crusty old pre-Hg code, so if we've got a better way to do the same thing, it would make sense to use it. Modifying the document when saving is pretty sucky. :(
git-blame tells me that this particular code goes all the way back to bug 46574, for reference.
Attached patch PatchSplinter Review
I'm at a loss for picking a reviewer, but the Embedding module has people associated with it, so let's start there.

https://treeherder.mozilla.org/#/jobs?repo=try&revision=7d42177b1de2
Attachment #8630083 - Flags: review?(jst)
Comment on attachment 8630083 [details] [diff] [review]
Patch

Yes, this stuff goes way way back... r=jst
Attachment #8630083 - Flags: review?(jst) → review+
Thanks for the review.

For checkin-needed, there's a Try run in comment #3 but also I forgot I'd done that one and did another: https://treeherder.mozilla.org/#/jobs?repo=try&revision=620a90bf01d4
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/77f5030f0742
Status: NEW → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 42
You need to log in before you can comment on or make changes to this bug.