No "Publishing completed" status after green tick, publishing does not terminate

RESOLVED FIXED in seamonkey2.46

Status

SeaMonkey
Composer
--
major
RESOLVED FIXED
2 years ago
2 years ago

People

(Reporter: Lee P, Assigned: frg)

Tracking

SeaMonkey 2.39 Branch
seamonkey2.46
Unspecified
All
Dependency tree / graph

SeaMonkey Tracking Flags

(seamonkey2.42 wontfix, seamonkey2.43 fixed, seamonkey2.44 fixed, seamonkey2.45 fixed, seamonkey2.46 fixed)

Details

Attachments

(2 attachments)

1.31 MB, application/vnd.openxmlformats-officedocument.wordprocessingml.document
Details
1.38 KB, patch
Philip Chee
: review+
Details | Diff | Splinter Review
(Reporter)

Description

2 years ago
Created attachment 8686985 [details]
Sea Monkey bug.docx

Publishing issue: Note evident until latest update.

File -> page edit -> publish -> green tick OK -> BUT CONTINUES TO PUBLISH. Only way to get out of the process is to cancel -> cancel -> cancel. When all cancelled can reload and all changes have been actioned.
Also the edit page is now an unknown entity - don't know if in edit page or not.
Please give some advice.
Thanks
Lee
Not a security bug.
Group: core-security-release

Comment 2

2 years ago
NOT reproducible REPRODUCIBLE with  SeaMonkey German 2.39 final Mozilla/5.0 (Windows NT 6.1; WOW64; rv:42.0 from official download area)  Gecko/20100101  Firefox/42.0  Build 20151103191810  (Classic Theme) on German WIN7 64bit with:

0. Download Attachment 8673077 [details] for Bug 1214005
1. Launch Composer → open file → add 1 character to first text line → click
   Publish-Icon'
   » Publish dialog appears
2. If necessary add user data for ftp upload → check "include images" in Publish
   TAB → [Publish]
   » Publishing to my ftp web space
   » after short moment green checkmark and status info "Publishing completed"
     appears

@Reporter: Please contribute additional information due to <https://developer.mozilla.org/en-US/docs/Mozilla/QA/Bug_writing_guidelines>  (containing every key press and every mouse click) how to reproduce the problem reliably:
a) Test document
b) free ftp upload account or similar
c) Results with safe modes (disabled add-ons) or newly created user profile?
d) until what version did that work for you?
Please answer without citing, simply start your reply like
a): <answer>
Severity: critical → major
Flags: needinfo?(mailman3981)
Summary: Publishing Not Functional as per previous Version → No "Publishing completed" after green tick, publishing does not terminate

Updated

2 years ago
Summary: No "Publishing completed" after green tick, publishing does not terminate → No "Publishing completed" status after green tick, publishing does not terminate

Comment 3

2 years ago
The faulty behaviour is also present in the Seamonkey packaged for Ubuntu: 2.39-0ubuntu1 (stable), http://ubuntuzilla.sourceforge.net/.

The problem seems to have come with version 2.39. My temporary solution is to use a 2.38 version instead, e g https://archive.mozilla.org/pub/mozilla.org/seamonkey/releases/2.38/contrib/seamonkey-2.38.en-US.linux-x86_64.tar.bz2
(Reporter)

Comment 4

2 years ago
From Lee P again.
Since my original posting and reverting back to 2.38 all is OK on my PC.
However we are now attempting to edit a website with SeaMonkey Composer 2.39 using an Apple Mac.
We expected that this faulty behaviour would not be evident on the Apple Mac.
However the same problem "[Bug 1224437] No "Publishing completed" status after green tick, publishing does not terminate" is eveident on the Apple Mac.

When will 2.39' be bug free of this problem?
Thanks
Lee
Flags: needinfo?(mailman3981)

Comment 5

2 years ago
OS = All due to comments.
Useful instruction still missing.
Any chance to get a test account for one of the ftp upload areas where the problem appears?
We need information due to comment 2.
Flags: needinfo?(mailman3981)
OS: Windows 10 → All
(Assignee)

Comment 6

2 years ago
I can confirm this too with a private Windows build.

User agent: Mozilla/5.0 (Windows NT 6.1; Win64; x64; rv:44.0) Gecko/20100101 Firefox/44.0 SeaMonkey/2.41 en-uUS

I traced it back to 

editor\ui\composer\content\ComposerCommands.js

>> // STATE_IS_NETWORK signals end of publishing, as does the gPersistObj.currentState
>> if (aStateFlags & nsIWebProgressListener.STATE_IS_NETWORK
>> && gPersistObj.currentState == nsIWebBrowserPersist.PERSIST_STATE_FINISHED)

Starting with 2.39 I never observed STATE_IS_NETWORK and PERSIST_STATE_FINISHED together so the if never evaluates to true. I am not sure if this isn't due to e10s changes in the backend. The comment indicated that both conditions signaled the end of publishing so as a test I used || instead of && which worked fine but I am more or less clueless about the apis involved so just consider it trial and error and not a fix.

FRG

Comment 7

2 years ago
I am able to duplicate this issue.  If necessary, I can provide a test ftp account to test with.
(Reporter)

Comment 8

2 years ago
From Lee P
Now that the bug has been duplicated by Chuck do you guys still need the info requested by Rainer?
Since I uninstalled 2.39 reinstalled 2.38 it has been impossible to re-create the 2.39 bug process. However if it's absolutely necessary I will upgrade to 2.39 and document the bug process for you.
Then I will uninstall 2.39 and reinstall 2.38 as the workaround.
Flags: needinfo?(mailman3981)

Comment 9

2 years ago
Frank-Rainer Grahl was able to reproduce the problem, so NEW, currently no more information required.
Status: UNCONFIRMED → NEW
Ever confirmed: true

Comment 10

2 years ago
I also observed the problem with SM 2.39 (?) few days ago, not remembering this Bug.

Comment 11

2 years ago
If you want this bug fixed, you need to vote it up at https://bugzilla.mozilla.org/show_bug.cgi?id=1224437 . As of now, there are only 3 votes (including mine) to get this fixed. No wonder it hasn't been fixed.
Comment hidden (advocacy)

Comment 13

2 years ago
I just signed up so that I could add my vote to get this bug squashed. 

I've gone back to version 2.38 (Mac) and the publishing is going smoothly again.

Alan S
(Assignee)

Comment 14

2 years ago
Created attachment 8741538 [details] [diff] [review]
1224437-state.patch

I am quite sure this was broken by the changes in Bug 1101100.

I think the PERSIST_STATE_FINISHED state can no longer be used bere because it is set after the state cange to STATE_IS_NETWORK in EndDownload 8which is actually an upload here...). The result is already set and only some cleanup remains so the code should not fail.


>> nsWebBrowserPersist::EndDownload(nsresult aResult)
>> {
>>  // Store the error code in the result if it is an error
>>  if (NS_SUCCEEDED(mPersistResult) && NS_FAILED(aResult))
>>  {
>>      mPersistResult = aResult;
>>  }
>>
>>  // State stop notification
>>  if (mProgressListener) {
>>      mProgressListener->OnStateChange(nullptr, nullptr,
>>          nsIWebProgressListener::STATE_STOP
>>          | nsIWebProgressListener::STATE_IS_NETWORK, mPersistResult);
>>  }
>>
>>    // Do file cleanup if required
>>    if (NS_FAILED(aResult) && (mPersistFlags & 
>>         PERSIST_FLAGS_CLEANUP_ON_FAILURE))
>>    {
>>        CleanupLocalFiles();
>>    }
>>
>>    // Cleanup the channels
>>    mCompleted = true;
>>    Cleanup();

Teste on c-c VS2015 x64 private en-US build.

Published one html with one image to an ftp site and to a local file destination. Both worked.

>> User agent: Mozilla/5.0 (Windows NT 6.1; Win64; x64; rv:48.0) Gecko/20100101
>> Firefox/48.0 SeaMonkey/2.45a1
>> Build identifier: 20160414231315

As stated I do not know much about the apis involved so it might break something else when publishing.
Assignee: nobody → frgrahl
Status: NEW → ASSIGNED
Attachment #8741538 - Flags: review?(philip.chee)
(Assignee)

Updated

2 years ago
Depends on: 1101100

Comment 15

2 years ago
(In reply to Frank-Rainer Grahl from comment #14)
> Created attachment 8741538 [details] [diff] [review]
> 1224437-state.patch
> 
> I am quite sure this was broken by the changes in Bug 1101100.
Or Followup Bug 1207864 https://hg.mozilla.org/mozilla-central/rev/44c1e75e7266
(I think Chatzilla is also affected)

> I think the PERSIST_STATE_FINISHED state can no longer be used here because
> it is set after the state change to STATE_IS_NETWORK in EndDownload which is
> actually an upload here...). The result is already set and only some cleanup
> remains so the code should not fail.
> 
> 
> >> nsWebBrowserPersist::EndDownload(nsresult aResult)
> >> {
> >>  // Store the error code in the result if it is an error
> >>  if (NS_SUCCEEDED(mPersistResult) && NS_FAILED(aResult))
> >>  {
> >>      mPersistResult = aResult;
> >>  }
> >>
> >>  // State stop notification
> >>  if (mProgressListener) {
> >>      mProgressListener->OnStateChange(nullptr, nullptr,
> >>          nsIWebProgressListener::STATE_STOP
> >>          | nsIWebProgressListener::STATE_IS_NETWORK, mPersistResult);
> >>  }
> >>
> >>    // Do file cleanup if required
> >>    if (NS_FAILED(aResult) && (mPersistFlags & 
> >>         PERSIST_FLAGS_CLEANUP_ON_FAILURE))
> >>    {
> >>        CleanupLocalFiles();
> >>    }
> >>
> >>    // Cleanup the channels
> >>    mCompleted = true;
> >>    Cleanup();
> 
> Teste on c-c VS2015 x64 private en-US build.
> 
> Published one html with one image to an ftp site and to a local file
> destination. Both worked.
> 
> >> User agent: Mozilla/5.0 (Windows NT 6.1; Win64; x64; rv:48.0) Gecko/20100101
> >> Firefox/48.0 SeaMonkey/2.45a1
> >> Build identifier: 20160414231315
> 
> As stated I do not know much about the apis involved so it might break
> something else when publishing.

NI: Jed Davis
Is it correct that we should stop checking for nsIWebBrowserPersist.PERSIST_STATE_FINISHED within onStateChange()?
Flags: needinfo?(jld)

Comment 16

2 years ago
Please check:
https://bugzilla.mozilla.org/showdependencytree.cgi?id=1101100&maxdepth=1&hide_resolved=0
For any other changes we need to port to SeaMonkey. Thanks.
Flags: needinfo?(neil)

Comment 17

2 years ago
(In reply to Philip Chee from comment #15)

> NI: Jed Davis
> Is it correct that we should stop checking for
> nsIWebBrowserPersist.PERSIST_STATE_FINISHED within onStateChange()?

No reply from Jed Davis
NI => wmccloskey@mozilla.com
Flags: needinfo?(wmccloskey)
(Sorry; still dealing with my backlog from being away last week.)

My intent in bug 1101100 was to change the behavior of nsWebBrowserPersist as little as possible while making it multiprocess-capable, but there's basically nothing in the way of unit tests to help ensure that, and the documentation on the various state/status flags is somewhat sparse and unclear.  So if anything changed as far as what flags are set, it's probably a bug in Gecko.  But it sounds like it might be easier to just change the frontend?
Flags: needinfo?(jld)
Flags: needinfo?(wmccloskey)

Comment 19

2 years ago
FRG: what if you backout http://hg.mozilla.org/mozilla-central/rev/44c1e75e7266 (Bug 1207864)
Flags: needinfo?(frgrahl)
(Assignee)

Comment 20

2 years ago
I can try but I am quite sure this wouldn't be a good idea because the result for mProgessListener would be undefined then. 

What should work is to put 

mCompleted = true;

before the state stop notification.
Flags: needinfo?(frgrahl)

Comment 21

2 years ago
> https://treeherder.mozilla.org/#/jobs?repo=try&revision=0a0b6368e2cbc774aea890250d23dad81392c9b2
(Assignee)

Comment 22

2 years ago
The treeherder build looks like it did ok. 

I tried it locally and it fixed it too. Reverted the patch and moved mCompleted before the start stop notification. A quick publish with an html and image to an ftp site worked ok.

Even if this is a gecko bug I would still drop checking for PERSIST_STATE_FINISHED as it seems that the flags are now really redundant here.

Updated

2 years ago
Depends on: 1269412

Comment 23

2 years ago
(In reply to Frank-Rainer Grahl from comment #22)
> The treeherder build looks like it did ok. 
> 
> I tried it locally and it fixed it too. Reverted the patch and moved
> mCompleted before the start stop notification. A quick publish with an html
> and image to an ftp site worked ok.
> 
> Even if this is a gecko bug I would still drop checking for
> PERSIST_STATE_FINISHED as it seems that the flags are now really redundant
> here.
PERSIST_STATE_FINISHED is used in several other places in ComposerCommands.js so I'm going to be conservative here. When Neil comes back we can ask him to decide.

Comment 24

2 years ago
Comment on attachment 8741538 [details] [diff] [review]
1224437-state.patch

r=me for comm- branches wherever Mozilla Bug 1269412 doesn't land.
Attachment #8741538 - Flags: review?(philip.chee) → review+
(Assignee)

Comment 25

2 years ago
https://hg.mozilla.org/releases/comm-beta/rev/304b2fd17d04
https://hg.mozilla.org/releases/comm-release/rev/4494632520a4

Didn't push to comm-esr45.
Status: ASSIGNED → RESOLVED
Last Resolved: 2 years ago
Resolution: --- → FIXED
(Assignee)

Updated

2 years ago
status-seamonkey2.42: --- → wontfix
status-seamonkey2.43: --- → fixed
status-seamonkey2.44: --- → fixed
status-seamonkey2.45: --- → fixed
status-seamonkey2.46: --- → fixed
Target Milestone: --- → seamonkey2.46

Updated

2 years ago
Flags: needinfo?(neil)
(Assignee)

Comment 26

2 years ago
Pushed to comm-esr45 as per todays status meeting:

https://hg.mozilla.org/releases/comm-esr45/rev/4ad1905323d8
You need to log in before you can comment on or make changes to this bug.