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

Application crash without exception - style missing in text #887

Closed
tony-abraham opened this issue Mar 29, 2022 · 2 comments
Closed

Application crash without exception - style missing in text #887

tony-abraham opened this issue Mar 29, 2022 · 2 comments
Labels
confirmed issue reported has been reproduced fixed - please verify Issue has been fixed. Please verify and close.

Comments

@tony-abraham
Copy link

Build environment: macOS
Target device: Moddable Two, desktop simulator

Description
For a Text PIU object without a style assigned, if an attempt is made to set the width, the application crashes without throwing any exceptions.
Mcsim crashes with the message "mcsim quit unexpectedly" (screenshot attached); Moddable Two attempts to reboot the application continuously.
It is understood that Text object needs a style if a width has to be set; but shouldn't an exception be thrown before the application crashes in case of a missing style?

Steps to Reproduce
Reproduced the issue on the following sample code using mcconfig -d -m

let application = new Application(null, {
	width: 320, height: 240,
	skin: new Skin({ fill: "white" })
});

/**
 * Setting a width to Text Object without a style causes an application crash without exception.
 * The crash without exception happens only if an attempt to set the width is done. 
 * Setting the height does not cause the crash
 */
let text = new Text(null, {
	//style: new Style({ font: "myFont" }),	
	width: 150, 
	height: 50,
	string: "sample string"
});
application.add(text);
export default application;

Expected behavior
It is understood that the Text object needs a style if width / string is to be set.
But, in case of an unforeseen error in an application logic which causes the style not to be set correctly, an attempt to set the width should throw an exception like "style missing in text" .

Images
image

Other information

  1. Text Object - Attempting to set a height without setting a style - does not cause a crash.
  2. Label Object - Attempting to set a width or height without setting a style - does not cause a crash
@tony-abraham tony-abraham changed the title Application crash without throwing an exception - style missing in text Application crash without exception - style missing in text Mar 29, 2022
@phoddie phoddie added the confirmed issue reported has been reproduced label Mar 29, 2022
@phoddie
Copy link
Collaborator

phoddie commented Mar 29, 2022

Thank you for the report. The absence of a layout should not trigger a crash.

in case of an unforeseen error in an application logic which causes the style not to be set correctly, an attempt to set the width should throw an exception like "style missing in text" .

Would you please explain how you concluded that throwing an exception from the Text constructor is the correct behavior?

For example, this works but would fail if the constructor were to throw in the absence of layout`:

let text = new Text(null, {
	width: 150, 
	height: 50,
	string: "sample string"
});
text.style = new Style({ font: "myFont" });
application.add(text);

This comment is unclear and doesn't appear to match the runtime behavior:

 * The crash without exception happens only if an attempt to set the width is done. 
 * Setting the height does not cause the crash

For example, this does not crash:

let text = new Text(null, {
	width: 0, 
	height: 50,
	string: "sample string"
});

Maybe the comment should be "The crash only occurs when width has a non-zero value"?

@tony-abraham
Copy link
Author

Thanks for the response.

Regarding the first point : Yes, I got your point. If the text constructor were to throw an exception, adding a style after creating the Text object (which works fine today) would also fail.
The expected behavior should be "The absence of a style property should not trigger a crash" (Similar to how label behaves today)

Regarding the comment : Yes, your statement is correct. The crash happens only when the width has a non zero value.

mkellner pushed a commit that referenced this issue May 26, 2022
@phoddie phoddie added the fixed - please verify Issue has been fixed. Please verify and close. label May 26, 2022
@phoddie phoddie closed this as completed Jun 30, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
confirmed issue reported has been reproduced fixed - please verify Issue has been fixed. Please verify and close.
Projects
None yet
Development

No branches or pull requests

2 participants