Skip to content

Issue/460editor content loss #4927

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
aforcier merged 3 commits into from
Dec 16, 2016
Merged

Issue/460editor content loss #4927

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
8c59b53
Remove old QuickMedia code that could be problematic
maxme Dec 16, 2016
943ef66
Fix an issue in the Hybrid editor that could lead to an empty post ti…
maxme Dec 16, 2016
e3baf0f
Fix EditorFragmentTest after adding getContent exception throw
maxme Dec 16, 2016
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
58 changes: 39 additions & 19 deletions WordPress/src/main/java/org/wordpress/android/ui/posts/EditPostActivity.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,7 @@
import org.wordpress.android.analytics.AnalyticsTracker;
import org.wordpress.android.analytics.AnalyticsTracker.Stat;
import org.wordpress.android.editor.EditorFragment;
import org.wordpress.android.editor.EditorFragment.IllegalEditorStateException;
import org.wordpress.android.editor.EditorFragmentAbstract;
import org.wordpress.android.editor.EditorFragmentAbstract.EditorFragmentListener;
import org.wordpress.android.editor.EditorFragmentAbstract.EditorDragAndDropListener;
Expand Down Expand Up @@ -360,7 +361,12 @@ public void run() {
new Thread(new Runnable() {
@Override
public void run() {
updatePostObject(true);
try {
updatePostObject(true);
} catch (IllegalEditorStateException e) {
AppLog.e(T.EDITOR, "Impossible to save the post, we weren't able to update it.");
return;
}
savePostToDb();
if (mHandler != null) {
mHandler.postDelayed(mAutoSave, AUTOSAVE_INTERVAL_MILLIS);
Expand Down Expand Up @@ -636,7 +642,12 @@ public void onClick(DialogInterface dialog, int id) {
new Thread(new Runnable() {
@Override
public void run() {
updatePostObject(false);
try {
updatePostObject(false);
} catch (IllegalEditorStateException e) {
AppLog.e(T.EDITOR, "Impossible to save and publish the post, we weren't able to update it.");
return;
}
savePostToDb();

// If the post is empty, don't publish
Expand Down Expand Up @@ -769,7 +780,7 @@ private void trackEditorCreatedPost(String action, Intent intent) {
);
}

private synchronized void updatePostObject(boolean isAutosave) {
private synchronized void updatePostObject(boolean isAutosave) throws IllegalEditorStateException {
if (mPost == null) {
AppLog.e(AppLog.T.POSTS, "Attempted to save an invalid Post.");
return;
Expand All @@ -795,7 +806,12 @@ private void savePostAsync(final AfterSavePostListener listener) {
new Thread(new Runnable() {
@Override
public void run() {
updatePostObject(false);
try {
updatePostObject(false);
} catch (IllegalEditorStateException e) {
AppLog.e(T.EDITOR, "Impossible to save the post, we weren't able to update it.");
return;
}
savePostToDb();
if (listener != null) {
listener.onPostSave();
Expand Down Expand Up @@ -844,7 +860,12 @@ private class SaveAndFinishTask extends AsyncTask<Void, Void, Boolean> {
@Override
protected Boolean doInBackground(Void... params) {
// Fetch post title and content from editor fields and update the Post object
updatePostObject(false);
try {
updatePostObject(false);
} catch (IllegalEditorStateException e) {
AppLog.e(T.EDITOR, "Impossible to save the post, we weren't able to update it.");
return false;
}

if (mEditorFragment != null && mPost.hasEmptyContentFields()) {
// new and empty post? delete it
Expand All @@ -865,7 +886,12 @@ protected Boolean doInBackground(Void... params) {
updatePostContentNewEditor(false, mPost.getTitle(), mPost.getContent());
savePostToDb();
} else {
updatePostObject(false);
try {
updatePostObject(false);
} catch (IllegalEditorStateException e) {
AppLog.e(T.EDITOR, "Impossible to save the post, we weren't able to update it.");
return false;
}
savePostToDb();
}
}
Expand Down Expand Up @@ -1307,7 +1333,7 @@ private void prepareMediaPost() {
/**
* Updates post object with content of this fragment
*/
public void updatePostContent(boolean isAutoSave) {
public void updatePostContent(boolean isAutoSave) throws IllegalEditorStateException {
Post post = getPost();

if (post == null) {
Expand Down Expand Up @@ -1624,11 +1650,6 @@ public void onActivityResult(int requestCode, int resultCode, Intent data) {
} catch (OutOfMemoryError e) {
AppLog.e(T.POSTS, e);
}
} else if (TextUtils.isEmpty(mEditorFragment.getContent())) {
// TODO: check if it was mQuickMediaType > -1
// Quick Photo was cancelled, delete post and finish activity
WordPress.wpDB.deletePost(getPost());
finish();
}
break;
case RequestCodes.VIDEO_LIBRARY:
Expand All @@ -1641,11 +1662,6 @@ public void onActivityResult(int requestCode, int resultCode, Intent data) {
if (!addMedia(capturedVideoUri)) {
ToastUtils.showToast(this, R.string.gallery_error, Duration.SHORT);
}
} else if (TextUtils.isEmpty(mEditorFragment.getContent())) {
// TODO: check if it was mQuickMediaType > -1
// Quick Photo was cancelled, delete post and finish activity
WordPress.wpDB.deletePost(getPost());
finish();
}
break;
}
Expand Down Expand Up @@ -1876,8 +1892,12 @@ private void handleGalleryImageUploadedLegacyEditor(Long galleryId, String local
// needed by the legacy editor to save local drafts
postContent = new SpannableStringBuilder(mEditorFragment.getSpannedContent());
} else {
postContent = new SpannableStringBuilder(StringUtils.notNullStr((String)
mEditorFragment.getContent()));
try {
postContent = new SpannableStringBuilder(StringUtils.notNullStr((String) mEditorFragment.getContent()));
} catch (IllegalEditorStateException e) {
AppLog.e(T.EDITOR, "Impossible to handle gallery upload, we weren't able to get content from the post");
return;
}
}
int selectionStart = 0;
int selectionEnd = postContent.length();
Expand Down
14 changes: 10 additions & 4 deletions ...WordPressEditor/src/androidTest/java/org.wordpress.android.editor/EditorFragmentTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,8 @@
import android.view.View;
import android.widget.ToggleButton;

import org.wordpress.android.editor.EditorFragment.IllegalEditorStateException;

import java.util.HashMap;
import java.util.Map;
import java.util.concurrent.CountDownLatch;
Expand Down Expand Up @@ -99,7 +101,7 @@ public void testFormatBarToggledOnSelectedFieldChanged() {
assertTrue(htmlButton.isEnabled());
}

public void testHtmlModeToggleTextTransfer() throws InterruptedException {
public void testHtmlModeToggleTextTransfer() throws InterruptedException, IllegalEditorStateException {
waitForOnDomLoaded();

final View view = mFragment.getView();
Expand Down Expand Up @@ -148,8 +150,12 @@ public void run() {
contentText.setText("new <b>content</b>");

// Check that getTitle() and getContent() return latest version even in HTML mode
assertEquals("new title", mFragment.getTitle());
assertEquals("new <b>content</b>", mFragment.getContent());
try {
assertEquals("new title", mFragment.getTitle());
assertEquals("new <b>content</b>", mFragment.getContent());
} catch (IllegalEditorStateException e) {
throw new RuntimeException();
}

htmlButton.performClick(); // Turn off HTML mode

Expand All @@ -174,4 +180,4 @@ private void waitForOnDomLoaded() {
}
}
}
}
}
37 changes: 28 additions & 9 deletions libs/editor/WordPressEditor/src/main/java/org/wordpress/android/editor/EditorFragment.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,11 @@
public class EditorFragment extends EditorFragmentAbstract implements View.OnClickListener, View.OnTouchListener,
OnJsEditorStateChangedListener, OnImeBackListener, EditorWebViewAbstract.AuthHeaderRequestListener,
EditorMediaUploadListener {

public class IllegalEditorStateException extends Exception {

}

private static final String ARG_PARAM_TITLE = "param_title";
private static final String ARG_PARAM_CONTENT = "param_content";

Expand Down Expand Up @@ -423,8 +428,12 @@ public void setUserVisibleHint(boolean isVisibleToUser) {

@Override
public void onSaveInstanceState(Bundle outState) {
outState.putCharSequence(KEY_TITLE, getTitle());
outState.putCharSequence(KEY_CONTENT, getContent());
try {
outState.putCharSequence(KEY_TITLE, getTitle());
outState.putCharSequence(KEY_CONTENT, getContent());
} catch (IllegalEditorStateException e) {
AppLog.e(T.EDITOR, "onSaveInstanceState: unable to get title or content");
}
}

private ActionBar getActionBar() {
Expand Down Expand Up @@ -636,9 +645,19 @@ public void run() {
}

// Update mTitle and mContentHtml with the latest state from the ZSSEditor
getTitle();
getContent();

try {
getTitle();
getContent();
} catch (IllegalEditorStateException e) {
AppLog.e(T.EDITOR, "toggleHtmlMode: unable to get title or content");

@aforcier aforcier Dec 16, 2016

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We also need to reset the toggle button state here with toggleButton.setChecked(false), otherwise it's left in a pressed state and might cause content loss issues when pressed again (as the HTML -> visual flow is triggered).

getActivity().runOnUiThread(new Runnable() {
@Override
public void run() {
toggleButton.setChecked(false);
}
});
return;
}
getActivity().runOnUiThread(new Runnable() {
@Override
public void run() {
Expand Down Expand Up @@ -922,9 +941,9 @@ public void setContent(CharSequence text) {
* where possible.
*/
@Override
public CharSequence getTitle() {
public CharSequence getTitle() throws IllegalEditorStateException {
if (!isAdded()) {
return "";
throw new IllegalEditorStateException();
}

if (mSourceView != null && mSourceView.getVisibility() == View.VISIBLE) {
Expand Down Expand Up @@ -961,9 +980,9 @@ public void run() {
* where possible.
*/
@Override
public CharSequence getContent() {
public CharSequence getContent() throws IllegalEditorStateException {
if (!isAdded()) {
return "";
throw new IllegalEditorStateException();
}

if (mSourceView != null && mSourceView.getVisibility() == View.VISIBLE) {
Expand Down
5 changes: 3 additions & 2 deletions ...or/WordPressEditor/src/main/java/org/wordpress/android/editor/EditorFragmentAbstract.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@

import com.android.volley.toolbox.ImageLoader;

import org.wordpress.android.editor.EditorFragment.IllegalEditorStateException;
import org.wordpress.android.util.helpers.MediaFile;
import org.wordpress.android.util.helpers.MediaGallery;

Expand All @@ -18,8 +19,8 @@
public abstract class EditorFragmentAbstract extends Fragment {
public abstract void setTitle(CharSequence text);
public abstract void setContent(CharSequence text);
public abstract CharSequence getTitle();
public abstract CharSequence getContent();
public abstract CharSequence getTitle() throws IllegalEditorStateException;
public abstract CharSequence getContent() throws IllegalEditorStateException;
public abstract void appendMediaFile(MediaFile mediaFile, String imageUrl, ImageLoader imageLoader);
public abstract void appendGallery(MediaGallery mediaGallery);
public abstract void setUrlForVideoPressId(String videoPressId, String url, String posterUrl);
Expand Down