Closed Bug 1269412 Opened 5 years ago Closed 5 years ago

In nsWebBrowserPersist::EndDownload set mCompleted = true before issuing the state stop notification

Categories

(Core Graveyard :: Embedding: APIs, defect)

defect
Not set
normal

Tracking

(firefox47 wontfix, firefox48 fixed, firefox49 fixed)

RESOLVED FIXED
mozilla49
Tracking Status
firefox47 --- wontfix
firefox48 --- fixed
firefox49 --- fixed

People

(Reporter: philip.chee, Assigned: philip.chee)

References

Details

Attachments

(1 file)

Bug 1101100 caused a regression in the SeaMonkey HTML Composer component. This can be fixed by moving |mCompleted = true| to before issuing the state stop notification.
Try Server Job:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=0a0b6368e2cbc774aea890250d23dad81392c9b2&group_state=expanded&exclusion_profile=false

Hopefully I didn't regress Bug 1101100. It doesn't seem to have any tests attached so I can't tell.
Attachment #8747797 - Flags: review?(jld)
Comment on attachment 8747797 [details] [diff] [review]
Move the mCompleted line before state stop

Review of attachment 8747797 [details] [diff] [review]:
-----------------------------------------------------------------

Looks reasonable.

As for testing, I tried it on my local tree, and it doesn't break the basic e10s Firefox “Save As…” functionality, at least.  (There are a few automated tests of nsWebBrowserPersist in embedding/test, but not remotely as many as there should be in my opinion, and the ones that are there don't always check results as much as they could.)
Attachment #8747797 - Flags: review?(jld) → review+
https://hg.mozilla.org/mozilla-central/rev/da1c47951c03
Status: ASSIGNED → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla49
Comment on attachment 8747797 [details] [diff] [review]
Move the mCompleted line before state stop

Approval Request Comment
[Feature/regressing bug #]: Bug 1101100 - [e10s] Make “Save As… Complete Document” work in e10s tabs

[User impact if declined]: Impacts only SeaMonkey and Chatzilla

[Describe test coverage new/current, TreeHerder]: https://treeherder.mozilla.org/#/jobs?repo=try&revision=0a0b6368e2cbc774aea890250d23dad81392c9b2&group_state=expanded&exclusion_profile=false

[Risks and why]: Risk is very low to none. Only SeaMonkey Composer component and Chatzilla bother to check the value returned. Nothing else does.

[String/UUID change made/needed]: None.
Attachment #8747797 - Flags: approval-mozilla-beta?
Attachment #8747797 - Flags: approval-mozilla-aurora?
Comment on attachment 8747797 [details] [diff] [review]
Move the mCompleted line before state stop

e10s will not ship on by default for 47, let's uplift this for Aurora48 only.
Attachment #8747797 - Flags: approval-mozilla-beta?
Attachment #8747797 - Flags: approval-mozilla-beta-
Attachment #8747797 - Flags: approval-mozilla-aurora?
Attachment #8747797 - Flags: approval-mozilla-aurora+
Product: Core → Core Graveyard
You need to log in before you can comment on or make changes to this bug.