Skip to content

Commit

Permalink
Show unadulterated HTML errors. (#2385)
Browse files Browse the repository at this point in the history
* Html errors.

* Correctly show errors when creating account.

* Don't show custom dialog for blocked errors.

* Regularize and optimize editing errors.

* Make all links from error go external.

* Add snackbar action for very long messages.

* Update link style within snackbar.

* Clean up.

Co-authored-by: Cooltey Feng <cfeng@wikimedia.org>
  • Loading branch information
dbrant and cooltey committed May 3, 2021
1 parent b47a373 commit 2ed8f8c
Show file tree
Hide file tree
Showing 13 changed files with 71 additions and 194 deletions.
Expand Up @@ -65,8 +65,10 @@ class CreateAccountActivity : BaseActivity() {
}

private fun setClickListeners() {
binding.viewCreateAccountError.backClickListener = View.OnClickListener { onBackPressed() }
binding.viewCreateAccountError.retryClickListener = View.OnClickListener { it.visibility = View.GONE }
binding.viewCreateAccountError.backClickListener = View.OnClickListener {
binding.viewCreateAccountError.visibility = View.GONE
}
binding.viewCreateAccountError.retryClickListener = View.OnClickListener { binding.viewCreateAccountError.visibility = View.GONE }
binding.createAccountSubmitButton.setOnClickListener {
validateThenCreateAccount()
}
Expand Down Expand Up @@ -155,11 +157,7 @@ class CreateAccountActivity : BaseActivity() {
}) { caught ->
L.e(caught.toString())
showProgressBar(false)
if (caught is CreateAccountException) {
handleAccountCreationError(caught.message!!)
} else {
showError(caught)
}
showError(caught)
})
}

Expand Down
2 changes: 1 addition & 1 deletion app/src/main/java/org/wikipedia/dataclient/Service.java
Expand Up @@ -40,7 +40,7 @@ public interface Service {
String COMMONS_URL = "https://commons.wikimedia.org/";
String URL_FRAGMENT_FROM_COMMONS = "/wikipedia/commons/";

String MW_API_PREFIX = "w/api.php?format=json&formatversion=2&errorformat=plaintext&";
String MW_API_PREFIX = "w/api.php?format=json&formatversion=2&errorformat=html&";

int PREFERRED_THUMB_SIZE = 320;

Expand Down
Expand Up @@ -14,14 +14,23 @@
public class MwServiceError implements ServiceError {
@SuppressWarnings("unused") @Nullable private String code;
@SuppressWarnings("unused") @Nullable private String text;
@SuppressWarnings("unused") @Nullable private String html;
@SuppressWarnings("unused") @Nullable private Data data;

public MwServiceError() {
}

public MwServiceError(@Nullable String code, @Nullable String html) {
this.code = code;
this.html = html;
}

@Override @NonNull public String getTitle() {
return StringUtils.defaultString(code);
}

@Override @NonNull public String getDetails() {
return StringUtils.defaultString(text);
return StringUtils.defaultString(html);
}

public boolean badToken() {
Expand Down
31 changes: 0 additions & 31 deletions app/src/main/java/org/wikipedia/edit/EditAbuseFilterResult.kt

This file was deleted.

100 changes: 8 additions & 92 deletions app/src/main/java/org/wikipedia/edit/EditSectionActivity.java
Expand Up @@ -12,7 +12,6 @@
import android.view.MenuItem;
import android.view.View;
import android.view.WindowManager;
import android.widget.ImageView;
import android.widget.ProgressBar;
import android.widget.ScrollView;
import android.widget.TextView;
Expand All @@ -21,9 +20,7 @@
import androidx.annotation.Nullable;
import androidx.appcompat.app.ActionBar;
import androidx.appcompat.app.AlertDialog;
import androidx.appcompat.content.res.AppCompatResources;

import org.apache.commons.lang3.StringUtils;
import org.wikipedia.Constants;
import org.wikipedia.R;
import org.wikipedia.WikipediaApp;
Expand All @@ -37,6 +34,7 @@
import org.wikipedia.dataclient.ServiceFactory;
import org.wikipedia.dataclient.mwapi.MwException;
import org.wikipedia.dataclient.mwapi.MwQueryPage;
import org.wikipedia.dataclient.mwapi.MwServiceError;
import org.wikipedia.edit.preview.EditPreviewFragment;
import org.wikipedia.edit.richtext.SyntaxHighlighter;
import org.wikipedia.edit.summaries.EditSummaryFragment;
Expand Down Expand Up @@ -85,10 +83,6 @@ public class EditSectionActivity extends BaseActivity {
@BindView(R.id.edit_keyboard_overlay) WikiTextKeyboardView wikiTextKeyboardView;
@BindView(R.id.view_edit_section_error) WikiErrorView errorView;
@BindView(R.id.view_progress_bar) ProgressBar progressBar;
@BindView(R.id.edit_section_abusefilter_container) View abusefilterContainer;
@BindView(R.id.edit_section_abusefilter_image) ImageView abuseFilterImage;
@BindView(R.id.edit_section_abusefilter_title) TextView abusefilterTitle;
@BindView(R.id.edit_section_abusefilter_text) TextView abusefilterText;
@BindView(R.id.captcha_container) View captchaInnerContainer;
@BindView(R.id.edit_section_captcha_container) View captchaContainer;

Expand All @@ -108,7 +102,6 @@ public class EditSectionActivity extends BaseActivity {
// Current revision of the article, to be passed back to the server to detect possible edit conflicts.
private long currentRevision;

private EditAbuseFilterResult abusefilterEditResult;
private CaptchaHandler captchaHandler;

private EditPreviewFragment editPreviewFragment;
Expand Down Expand Up @@ -291,12 +284,12 @@ private void doSave(@NonNull String token) {
if (result.hasEditResult() && result.edit() != null) {
if (result.edit().editSucceeded()) {
onEditSuccess(new EditSuccessResult(result.edit().newRevId()));
} else if (result.edit().hasEditErrorCode()) {
onEditSuccess(new EditAbuseFilterResult(result.edit().code(), result.edit().info(), result.edit().warning()));
} else if (result.edit().hasSpamBlacklistResponse()) {
onEditSuccess(new EditSpamBlacklistResult(result.edit().spamblacklist()));
} else if (result.edit().hasCaptchaResponse()) {
onEditSuccess(new CaptchaResult(result.edit().captchaId()));
} else if (result.edit().hasSpamBlacklistResponse()) {
onEditFailure(new MwException(new MwServiceError(result.edit().code(), result.edit().spamblacklist())));
} else if (result.edit().hasEditErrorCode()) {
onEditFailure(new MwException(new MwServiceError(result.edit().code(), result.edit().info())));
} else {
onEditFailure(new IOException("Received unrecognized edit response"));
}
Expand Down Expand Up @@ -327,16 +320,6 @@ public void onEditSuccess(@NonNull EditResult result) {
captchaContainer.setVisibility(View.VISIBLE);
captchaHandler.handleCaptcha(null, (CaptchaResult) result);
funnel.logCaptchaShown();
} else if (result instanceof EditAbuseFilterResult) {
abusefilterEditResult = (EditAbuseFilterResult) result;
handleAbuseFilter();
if (abusefilterEditResult.getType() == EditAbuseFilterResult.TYPE_ERROR) {
editPreviewFragment.hide();
}
} else if (result instanceof EditSpamBlacklistResult) {
FeedbackUtil.showMessage(EditSectionActivity.this,
R.string.editing_error_spamblacklist);
editPreviewFragment.hide();
} else {
funnel.logError(result.getResult());
// Expand to do everything.
Expand Down Expand Up @@ -375,34 +358,7 @@ private void showRetryDialog(@NonNull Throwable t) {
private void handleEditingException(@NonNull MwException caught) {
String code = caught.getTitle();

if (StringUtils.defaultString(code).contains("abusefilter")) {
abusefilterEditResult = new EditAbuseFilterResult(code, caught.getMessage(), caught.getMessage());
handleAbuseFilter();
if (abusefilterEditResult.getType() == EditAbuseFilterResult.TYPE_ERROR) {
editPreviewFragment.hide();
}
} else if ("blocked".equals(code) || "wikimedia-globalblocking-ipblocked".equals(code)) {
// User is blocked, locally or globally
// If they were anon, canedit does not catch this, so we can't show them the locked pencil
// If they not anon, this means they were blocked in the interim between opening the edit
// window and clicking save. Less common, but might as well handle it
AlertDialog.Builder builder = new AlertDialog.Builder(EditSectionActivity.this);
builder.setTitle(R.string.user_blocked_from_editing_title);
if (AccountUtil.isLoggedIn()) {
builder.setMessage(R.string.user_logged_in_blocked_from_editing);
builder.setPositiveButton(R.string.account_editing_blocked_dialog_ok_button_text, (dialog, i) -> dialog.dismiss());
} else {
builder.setMessage(R.string.user_anon_blocked_from_editing);
builder.setPositiveButton(R.string.nav_item_login, (dialog, i) -> {
dialog.dismiss();
Intent loginIntent = LoginActivity.newIntent(EditSectionActivity.this,
LoginFunnel.SOURCE_BLOCKED);
startActivity(loginIntent);
});
builder.setNegativeButton(R.string.account_editing_blocked_dialog_cancel_button_text, (dialog, i) -> dialog.dismiss());
}
builder.show();
} else if ("editconflict".equals(code)) {
if ("editconflict".equals(code)) {
new AlertDialog.Builder(EditSectionActivity.this)
.setTitle(R.string.edit_conflict_title)
.setMessage(R.string.edit_conflict_message)
Expand All @@ -414,32 +370,6 @@ private void handleEditingException(@NonNull MwException caught) {
}
}

private void handleAbuseFilter() {
if (abusefilterEditResult == null) {
return;
}
if (abusefilterEditResult.getType() == EditAbuseFilterResult.TYPE_ERROR) {
funnel.logAbuseFilterError(abusefilterEditResult.getCode());
abuseFilterImage.setImageDrawable(AppCompatResources.getDrawable(this, R.drawable.ic_abusefilter_disallow));
abusefilterTitle.setText(getString(R.string.abusefilter_title_disallow));
abusefilterText.setText(StringUtil.fromHtml(getString(R.string.abusefilter_text_disallow)));
} else {
funnel.logAbuseFilterWarning(abusefilterEditResult.getCode());
abuseFilterImage.setImageDrawable(AppCompatResources.getDrawable(this, R.drawable.ic_abusefilter_warn));
abusefilterTitle.setText(getString(R.string.abusefilter_title_warn));
abusefilterText.setText(StringUtil.fromHtml(getString(R.string.abusefilter_text_warn)));
}

hideSoftKeyboard(this);
ViewAnimations.fadeIn(abusefilterContainer, this::supportInvalidateOptionsMenu);
}


private void cancelAbuseFilter() {
abusefilterEditResult = null;
ViewAnimations.fadeOut(abusefilterContainer, this::supportInvalidateOptionsMenu);
}

/**
* Executes a click of the actionbar button, and performs the appropriate action
* based on the current state of the button.
Expand All @@ -452,10 +382,6 @@ public void clickNextButton() {
editPreviewFragment.setCustomSummary(editSummaryFragment.getSummary());
} else if (editPreviewFragment.isActive()) {
//we're showing the Preview window, which means that the next step is to save it!
if (abusefilterEditResult != null) {
//if the user was already shown an AbuseFilter warning, and they're ignoring it:
funnel.logAbuseFilterWarningIgnore(abusefilterEditResult.getCode());
}
getEditTokenThenSave();
funnel.logSaveAttempt();
} else {
Expand Down Expand Up @@ -498,11 +424,7 @@ public boolean onCreateOptionsMenu(Menu menu) {

item.setTitle(getString(editPreviewFragment.isActive() ? R.string.edit_done : R.string.edit_next));
if (progressBar.getVisibility() == View.GONE) {
if (abusefilterEditResult != null) {
item.setEnabled(abusefilterEditResult.getType() != EditAbuseFilterResult.TYPE_ERROR);
} else {
item.setEnabled(sectionTextModified);
}
item.setEnabled(sectionTextModified);
} else {
item.setEnabled(false);
}
Expand All @@ -529,6 +451,7 @@ public void onActionModeStarted(ActionMode mode) {
}

public void showError(@Nullable Throwable caught) {
hideSoftKeyboard(this);
errorView.setError(caught);
errorView.setVisibility(View.VISIBLE);
}
Expand Down Expand Up @@ -664,13 +587,6 @@ public void onBackPressed() {
captchaHandler.cancelCaptcha();
captchaContainer.setVisibility(View.GONE);
}
if (abusefilterEditResult != null) {
if (abusefilterEditResult.getType() == EditAbuseFilterResult.TYPE_WARNING) {
funnel.logAbuseFilterWarningBack(abusefilterEditResult.getCode());
}
cancelAbuseFilter();
return;
}
if (errorView.getVisibility() == View.VISIBLE) {
errorView.setVisibility(View.GONE);
}
Expand Down

This file was deleted.

1 change: 1 addition & 0 deletions app/src/main/java/org/wikipedia/history/HistoryEntry.java
Expand Up @@ -45,6 +45,7 @@ public class HistoryEntry implements Parcelable {
public static final int SOURCE_TALK_TOPIC = 31;
public static final int SOURCE_WATCHLIST = 32;
public static final int SOURCE_EDIT_DIFF_DETAILS = 33;
public static final int SOURCE_ERROR = 34;

@NonNull private final PageTitle title;
@NonNull private final Date timestamp;
Expand Down
3 changes: 2 additions & 1 deletion app/src/main/java/org/wikipedia/page/PageViewModel.java
Expand Up @@ -71,7 +71,8 @@ public boolean shouldForceNetwork() {

public boolean shouldLoadAsMobileWeb() {
return (title != null && (title.namespace() == Namespace.SPECIAL || title.isMainPage()))
|| (page != null && (page.getPageProperties().getNamespace() != Namespace.MAIN || page.isMainPage()));
|| (page != null && ((page.getPageProperties().getNamespace() != Namespace.MAIN
&& page.getPageProperties().getNamespace() != Namespace.USER) || page.isMainPage()));
}

public void setWatched(boolean isWatched) {
Expand Down
13 changes: 12 additions & 1 deletion app/src/main/java/org/wikipedia/util/FeedbackUtil.kt
Expand Up @@ -26,6 +26,7 @@ import org.wikipedia.page.PageActivity
import org.wikipedia.page.PageTitle
import org.wikipedia.random.RandomActivity
import org.wikipedia.readinglist.ReadingListActivity
import org.wikipedia.richtext.RichTextUtil
import org.wikipedia.staticdata.SpecialAliasData
import org.wikipedia.staticdata.UserAliasData
import org.wikipedia.suggestededits.SuggestionsActivity
Expand All @@ -47,7 +48,15 @@ object FeedbackUtil {
@JvmStatic
fun showError(activity: Activity, e: Throwable?) {
val error = ThrowableUtil.getAppError(activity, e!!)
makeSnackbar(activity, error.error, LENGTH_DEFAULT).show()
makeSnackbar(activity, error.error, LENGTH_DEFAULT).also {
if (error.error.length > 200) {
it.duration = Snackbar.LENGTH_INDEFINITE
it.setAction(android.R.string.ok) { _ ->
it.dismiss()
}
}
it.show()
}
}

@JvmStatic
Expand Down Expand Up @@ -151,7 +160,9 @@ object FeedbackUtil {
val view = findBestView(activity)
val snackbar = Snackbar.make(view, StringUtil.fromHtml(text.toString()), duration)
val textView = snackbar.view.findViewById<TextView>(R.id.snackbar_text)
textView.setLinkTextColor(ResourceUtil.getThemedColor(view.context, R.attr.color_group_52))
textView.movementMethod = LinkMovementMethod.getInstance()
RichTextUtil.removeUnderlinesFromLinks(textView)
val actionView = snackbar.view.findViewById<TextView>(R.id.snackbar_action)
actionView.setTextColor(ResourceUtil.getThemedColor(view.context, R.attr.color_group_52))
return snackbar
Expand Down

0 comments on commit 2ed8f8c

Please sign in to comment.