Skip to content
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

Jump to bottom

Fixes https://github.com/react-native-webview/react-native-webview/is… #3307

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Fixes https://github.com/react-native-webview/react-native-webview/is… #3307

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
35 changes: 23 additions & 12 deletions android/src/newarch/com/reactnativecommunity/webview/RNCWebViewManager.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -420,39 +420,46 @@ public void setUserAgent(RNCWebViewWrapper view, @Nullable String value) {
mRNCWebViewManagerImpl.setUserAgent(view, value);
}

// These will never be called because we use the shared impl for now
@Override
public void goBack(RNCWebViewWrapper view) {
view.getWebView().goBack();
// NOTE: It is handled via receiveCommand().
// view.getWebView().goBack();
}

@Override
public void goForward(RNCWebViewWrapper view) {
view.getWebView().goForward();
// NOTE: It is handled via receiveCommand().
// view.getWebView().goForward();
}

@Override
public void reload(RNCWebViewWrapper view) {
view.getWebView().reload();
// NOTE: It is handled via receiveCommand().
// view.getWebView().reload();
}

@Override
public void stopLoading(RNCWebViewWrapper view) {
view.getWebView().stopLoading();
// NOTE: It is handled via receiveCommand().
// view.getWebView().stopLoading();
}

@Override
public void injectJavaScript(RNCWebViewWrapper view, String javascript) {
view.getWebView().evaluateJavascriptWithFallback(javascript);
// NOTE: It is handled via receiveCommand().
// view.getWebView().evaluateJavascriptWithFallback(javascript);
}

@Override
public void requestFocus(RNCWebViewWrapper view) {
view.requestFocus();
// NOTE: It is handled via receiveCommand().
// view.requestFocus();
Copy link

Choose a reason for hiding this comment

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

What is the purpose of leaving empty overrides?

Copy link
Author

Choose a reason for hiding this comment

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

@o-alexandrov I don't have previous experience with native UI components, and I haven't looked much into how exactly that codegenNativeCommands() and .receiveCommands() stuff works. Just looking at the existing code and behavior it looked to me that if these .requestFocus(), etc. methods get called directly, perhaps they are the best way to implement related functionality (and probably they are abstract in a parent class, thus have to be overridden anyway — but I haven't really checked it). Then, there is the backward compatibility to the old arch, I thought if I touch .receiveCommands(), perhaps it is needed for the old arch, and it will need a bunch more efforts to ensure it works there.

My immediate interest here was just to hotfix the bug causing me troubles, thus I figured out this is the minimal intrusion way to do it — comment out the calls that cause duplicated command executions, but otherwise leave these method in the codebase so that somebody later looks second time into it.

}

@Override
public void postMessage(RNCWebViewWrapper view, String data) {
// NOTE: It is handled via receiveCommand().
/*
try {
JSONObject eventInitDict = new JSONObject();
eventInitDict.put("data", data);
Expand All @@ -472,28 +479,32 @@ public void postMessage(RNCWebViewWrapper view, String data) {
} catch (JSONException e) {
throw new RuntimeException(e);
}
*/
}

@Override
public void loadUrl(RNCWebViewWrapper view, String url) {
view.getWebView().loadUrl(url);
// NOTE: It is handled via receiveCommand().
// view.getWebView().loadUrl(url);
}

@Override
public void clearFormData(RNCWebViewWrapper view) {
view.getWebView().clearFormData();
// NOTE: It is handled via receiveCommand().
// view.getWebView().clearFormData();
}

@Override
public void clearCache(RNCWebViewWrapper view, boolean includeDiskFiles) {
view.getWebView().clearCache(includeDiskFiles);
// NOTE: It is handled via receiveCommand().
// view.getWebView().clearCache(includeDiskFiles);
}

@Override
public void clearHistory(RNCWebViewWrapper view) {
view.getWebView().clearHistory();
// NOTE: It is handled via receiveCommand().
// view.getWebView().clearHistory();
}
// !These will never be called

@Override
protected void addEventEmitters(@NonNull ThemedReactContext reactContext, RNCWebViewWrapper view) {
Expand Down