Closed
Bug 1179967
Opened 9 years ago
Closed 9 years ago
Save As (nsWebBrowserPersist) shouldn't modify the document
Categories
(Firefox :: General, defect)
Firefox
General
Tracking
()
RESOLVED
FIXED
Firefox 42
| Tracking | Status | |
|---|---|---|
| firefox42 | --- | fixed |
People
(Reporter: jld, Assigned: jld)
References
Details
Attachments
(1 file)
|
11.85 KB,
patch
|
jst
:
review+
|
Details | Diff | Splinter Review |
“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).
|
||
Comment 1•9 years ago
|
||
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. :(
|
Assignee | |
Comment 2•9 years ago
|
||
git-blame tells me that this particular code goes all the way back to bug 46574, for reference.
|
|
Assignee | |
Comment 3•9 years ago
|
||
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 4•9 years ago
|
||
Comment on attachment 8630083 [details] [diff] [review]
Patch
Yes, this stuff goes way way back... r=jst
Attachment #8630083 -
Flags: review?(jst) → review+
|
|
Assignee | |
Comment 5•9 years ago
|
||
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
Keywords: checkin-needed
|
||
Comment 7•9 years ago
|
||
Status: NEW → RESOLVED
Closed: 9 years ago
status-firefox42:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 42
You need to log in
before you can comment on or make changes to this bug.
Description
•