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.

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

Build 164: Closing and re-opening a-Shell -> black screen #235

Closed
personalizedrefrigerator opened this issue Jun 5, 2021 · 27 comments
Closed

Comments

@personalizedrefrigerator
Copy link
Contributor

Summary

  • After closing and re-opening a-Shell, the screen is black, and commands I enter seem to do nothing.
    • When no external keyboard is connected, the touch keyboard does not appear.

Terminal settings

  • A rather large font-size (config -s 16, I think)
  • Black background (config -b black)
  • White foreground

Thanks again for working on this!

@personalizedrefrigerator
Copy link
Contributor Author

personalizedrefrigerator commented Jun 5, 2021

If I call setupHterm() myself after starting a-Shell (adding sleep 4 \n jsi -wc "setupHterm();" to .profile) the terminal works as expected (but doesn't apply my custom font size or background).

@holzschu
Copy link
Owner

holzschu commented Jun 5, 2021

Looking at the debugging console, I see that this line:

command = "window.term_.prefs_.set('foreground-color', '" + foregroundColor.toHexString() + "'); window.term_.prefs_.set('background-color', '" + backgroundColor.toHexString() + "'); window.term_.prefs_.set('cursor-color', '" + cursorColor.toHexString() + "'); window.term_.prefs_.set('font-size', '\(fontSize)'); window.term_.prefs_.set('font-family', '\(fontName)');"

results in an error: "JavaScript execution returned a result of an unsupported type". There is a strong probability that the two problems are connected.

@kkebo
Copy link
Contributor

kkebo commented Jun 5, 2021

Same here. (a-Shell 1.7.3 (164), iPadOS 14.7 DB 2)

In my case, opening a new window in Split View fixes the issue temporarily. (iPadOS only)

@holzschu
Copy link
Owner

holzschu commented Jun 5, 2021

I've traced it to lib.PreferenceManager.prototype.set which now, with the new hterm version, returns a Promise.resolve(), which is (unsurprisingly) not handled by webView.evaluateJavaScript(). I've created a synchronous version, which is what we now call from the Swift side. The repository is updated now; the build will take a bit longer.

@holzschu
Copy link
Owner

holzschu commented Jun 6, 2021

Build 165 is out on TestFlight. It seems to solve the issue on my setup.

@personalizedrefrigerator
Copy link
Contributor Author

Build 165 is out on TestFlight. It seems to solve the issue on my setup.

I'm still experiencing the issue...

@holzschu
Copy link
Owner

holzschu commented Jun 6, 2021

It seems to works when running from Xcode but not when running from TestFlight. This might be difficult to debug.

@kkebo
Copy link
Contributor

kkebo commented Jun 6, 2021

I've found that the following change would fix the problem, so I think it is something like a race condition.

diff --git a/script.js b/script.js
index e3c3f5ac..c288568c 100644
--- a/script.js
+++ b/script.js
@@ -990,7 +990,7 @@ function setupHterm() {
 
 window.onload = function() {
        lib.init();
-       setupHterm();
+    setTimeout(() => setupHterm(), 3000);
 };
 
 // Modifications and additions to hterm_all.js:

@personalizedrefrigerator
Copy link
Contributor Author

personalizedrefrigerator commented Jun 6, 2021

3B562FB8-097B-421F-8E44-28CCADDA0F9E
It looks like lib.init is async...

Maybe

window.onload = function() {
    lib.init().then(() => setupHterm());
};

@kkebo
Copy link
Contributor

kkebo commented Jun 6, 2021

I tried lib.init().then(() => setupHterm()) and await lib.init(), but it doesn't seem that lib.init relates to at least setupHterm().

On the other hand, this modification will fix the issue:

diff --git a/script.js b/script.js
index e3c3f5ac..5cd06a27 100644
--- a/script.js
+++ b/script.js
@@ -337,6 +337,7 @@ function updateAutocompleteMenu(io, cursorPosition) {
 }
 
 function setupHterm() {
+    setTimeout(() => {
 	const term = new hterm.Terminal();
 	// Default monospaced fonts installed: Menlo and Courier. 
 	term.prefs_.set('cursor-shape', 'BLOCK'); 
@@ -986,6 +987,7 @@ function setupHterm() {
 		window.commandRunning = window.commandToExecute;
 		window.commandToExecute = ""; 
 	}
+    }, 3000);
 }
 
 window.onload = function() {

But this one won't:

diff --git a/script.js b/script.js
index e3c3f5ac..40f8c3c4 100644
--- a/script.js
+++ b/script.js
@@ -338,6 +338,7 @@ function updateAutocompleteMenu(io, cursorPosition) {
 
 function setupHterm() {
 	const term = new hterm.Terminal();
+    setTimeout(() => {
 	// Default monospaced fonts installed: Menlo and Courier. 
 	term.prefs_.set('cursor-shape', 'BLOCK'); 
 	term.prefs_.set('font-family', window.fontFamily);
@@ -986,6 +987,7 @@ function setupHterm() {
 		window.commandRunning = window.commandToExecute;
 		window.commandToExecute = ""; 
 	}
+    }, 3000);
 }
 
 window.onload = function() {

Therefore, the timing at which new hterm.Terminal() is called seems to be important.

@holzschu
Copy link
Owner

holzschu commented Jun 6, 2021

The hterm documentation now recommends:

window.onload = async function() {
  await lib.init();
  setupHterm();
};

I'm going to try with that.

@kkebo
Copy link
Contributor

kkebo commented Jun 6, 2021

@holzschu I already tried that. This issue seems to be caused by another reason.

@holzschu
Copy link
Owner

holzschu commented Jun 6, 2021

@kkk669 thanks for the warning. I'll try with the timeout then. But I'm curious: how do you manage to reproduce the issue when you're compiling the app yourself? For me it always works when installed from Xcode.

@kkebo
Copy link
Contributor

kkebo commented Jun 6, 2021

@holzschu I reproduce the issue by the following steps.

  1. Modify source code
  2. Build and run the app on the device (from Xcode)
  3. Kill it
  4. Reopen it from the home screen on the device directly (this will reproduce the issue)

@holzschu
Copy link
Owner

holzschu commented Jun 6, 2021

Thanks. With your instructions, I am able to reproduce the issue. I'd like to avoid the timeout, as it might break later. await does not really wait, as you found out. But waiting for hterm.windowType to be defined does work:

  var interval = setInterval(function() {
    if (hterm.windowType) {
      clearInterval(interval);
      setupHterm();
    }
  }, 200);

Now that works (as in, I get the prompt), but the session setup has not been done, so the colors and text are lost. But I feel I'm getting there.

@kkebo
Copy link
Contributor

kkebo commented Jun 6, 2021

@holzschu

I'd like to avoid the timeout

Me too. I had used it to isolate the causes.

But waiting for hterm.windowType to be defined does work:

hterm.windowType is defined in this function and it is awaited here, so I believe it will be defined until await lib.init() returns. Am I missing anything? 🤔

@holzschu
Copy link
Owner

holzschu commented Jun 6, 2021

I was thinking that it was possible that await lib.init() returns before lib.init() is complete (it returns really quickly).
But the fix doesn't work actually.
If lib.init() returns and is complete, then why is new hterm.Terminal() taking forever? It calls onTerminalReady, so it could be that.

@kkebo
Copy link
Contributor

kkebo commented Jun 6, 2021

It calls onTerminalReady, so it could be that.

Are you referring to this line? When I inserted a delay to there (inside or outside of the closure), it didn't give any effects.

@holzschu
Copy link
Owner

holzschu commented Jun 6, 2021

So the signs are pointing towards hterm.Terminal = function({profileId} = {}).
Using your setTimeout(() => { in setupHterm made me realise that the previous window content was copied twice, so at least I could fix that.

@holzschu
Copy link
Owner

holzschu commented Jun 6, 2021

With setTimeOut on the second line of setupHterm, as in your second example, the terminal does not start even when called from Xcode. Does that ring a bell fo you?

@kkebo
Copy link
Contributor

kkebo commented Jun 6, 2021

With setTimeOut on the second line of setupHterm, as in your second example, the terminal does not start even when called from Xcode. Does that ring a bell fo you?

Yes. setTimeout should be on the first line of setupHterm not the second line as I wrote (Sorry for the confusion caused by my poor English). This is because new hterm.Terminal() have to be delayed. But I don't know why it have to be delayed. If we could figure that out, I think we could find another solution.

@holzschu
Copy link
Owner

holzschu commented Jun 6, 2021

Thanks.
I placed a test on the first line of setupHterm:

	if (!hterm.Terminal) { ... }

and it was triggered. It seems that sometimes, lib.init() is done, but hterm.Terminal still does not exist. I will try waiting for hterm.Terminal.

@personalizedrefrigerator
Copy link
Contributor Author

personalizedrefrigerator commented Jun 6, 2021

I'm not sure if this is helpful at this point, but these are the errors I'm getting from setupHterm() when called too early:
Error processing accessibility handler

Stack trace continued

From the stack trace, it looks like the issue relates to this:
Tries to print before everything is set up
Calling function on terminal resize

(Gotten by adding

# Reference: https://stackoverflow.com/questions/12571650/catching-all-javascript-unhandled-exceptions
jsi -wc 'window.addEventListener("error", (e) => { alert("Error: " + e.error.message + "\nStack:" + e.error.stack); });'
jsi -wc 'setupHterm();'

to the beginning of my .profile.)

@holzschu
Copy link
Owner

holzschu commented Jun 6, 2021

I think part of the issues may be caused by calling setupHterm() a second time. Still, I've modified print so that it buffers everything until accessibilityReader_ is not null:

hterm.Terminal.IO.prototype.print =
hterm.Terminal.IO.prototype.writeUTF16 = function(string) {
  // If another process has the foreground IO, buffer new data sent to this IO
  // (since it's in the background).  When we're made the foreground IO again,
  // we'll flush everything.
  // iOS addition: don't print until accessibilityReader_ is available. buffer strings until then.
  if ((this.terminal_.io != this) || (window.term_.accessibilityReader_ == null)) {
    this.buffered_ += string;
    return;
  }

  this.terminal_.interpret(string);
  // iOS: keep a copy of everything that has been sent to the screen, 
  // to restore terminal status later and resize.
  if (this.terminal_.isPrimaryScreen()) {
     window.printedContent += string;
  }
};

@holzschu
Copy link
Owner

holzschu commented Jun 6, 2021

Inspired by your idea, I've added window.onerror in script.js, which sends the error to the Swift side, which saves it to a file (alerts and console.log were not working):

window.onerror = (msg, url, line, column, error) => {
  const message = {
    message: msg,
    url: url,
    line: line,
    column: column,
    error: JSON.stringify(error)
  }

  if (window.webkit) {
   window.webkit.messageHandlers.aShell.postMessage('JS Error:' + msg + ' ' + error.stack);
  } else {
    console.log("Error:", message);
  }
};

@holzschu
Copy link
Owner

holzschu commented Jun 9, 2021

I think it's fixed in build 166, but I'll wait for your tests.

@personalizedrefrigerator
Copy link
Contributor Author

Yes! It's fixed!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants