smartdevicelink / sdl_evolution

Tracking and proposing changes to SDL's public APIs.
https://smartdevicelink.github.io/sdl_evolution/
BSD 3-Clause "New" or "Revised" License
33 stars 122 forks source link

[Accepted with Revisions] SDL 0335 - Limit TextField Length According to HMI Capabilities #1142

Closed theresalech closed 3 years ago

theresalech commented 3 years ago

Hello SDL community,

The review of "SDL 0335 - Limit TextField Length According to HMI Capabilities" begins now and runs through May 18, 2021. The proposal is available here:

https://github.com/smartdevicelink/sdl_evolution/blob/master/proposals/0335-limit-textfield-length-according-to-hmi-capabilities.md

Reviews are an important part of the SDL evolution process. All reviews should be sent to the associated Github issue at:

https://github.com/smartdevicelink/sdl_evolution/issues/1142

What goes into a review?

The goal of the review process is to improve the proposal under review through constructive criticism and, eventually, determine the direction of SDL. When writing your review, here are some questions you might want to answer in your review:

More information about the SDL evolution process is available at

https://github.com/smartdevicelink/sdl_evolution/blob/master/process.md

Thank you, Theresa Lech

Program Manager - Livio theresa@livio.io

joeljfischer commented 3 years ago

1.

When Core is evaluating a TextFieldStruct for truncation, it will first check if the length of the provided TextFieldStruct.fieldText is longer than the maximum length from the capabilities. The maximum length from the capabilities will be calculated as TextField.width * TextField.rows. If the provided fieldText is too long, the string will be cut off at the maximum length.

I don't know that multiplying width by rows is appropriate. I assume that rows are created with \n newlines? If so, shouldn't this be looking for those newlines, then checking each row by the width and truncating each row?

2.

Optionally, a suffix for the truncated data may also be defined in the INI configuration file.

Some languages write RTL, such as Arabic and Hebrew, and appending a suffix isn't appropriate.

This leaves us with a few possible solutions:

  1. Only allow automatic suffix truncation, the current solution.
  2. Add a prefix truncation in addition to suffix truncation.

However, there are additional problems with both of these solutions in the proposal as designed. All languages may not understand "..." as a truncation mechanism (it's actually surprisingly hard to find information about this), but this proposal provides no way to customize this for different languages without turning it off and developing the HMI to do it. This leads me to question the overall value of this proposal for any OEM that supports truncation in multiple languages. We can still do it to make it easier for English-only and other languages that support a single style of truncation, but that seems much less useful as a Core addition to me compared to providing good example truncation in the generic_hmi.

3.

When Core has truncated a TextField, the RPC response should make mobile aware that truncation has occurred. A string such as "$textFieldName was truncated." should be appended to the response's info and if the response code will be SUCCESS, it should be overwritten to WARNINGS.

I think that this change would be good, even if the rest of the proposal was not implemented, though (if the rest of the proposal were not implemented), I think the string would probably change to something like "$textFieldName is longer than the accepted length and it may have been truncated."

Overall, I'm not sure that this is a good change to Core due to its lack of flexibility, even if we added prefix truncation, and because I suspect it will not be used in production by most OEMs.

iCollin commented 3 years ago

1:

I don't know that multiplying width by rows is appropriate. I assume that rows are created with \n newlines? If so, shouldn't this be looking for those newlines, then checking each row by the width and truncating each row?

This is certainly an alternate solution. The reason I proposed to not parse out the rows in Core is because it is much more permissive to have the HMI wrap lines. It will require less code in each app to adjust the message for each different HMI, but apps will still have the ability to do this if they choose.

2.5:

All languages may not understand "..." as a truncation mechanism (it's actually surprisingly hard to find information about this), but this proposal provides no way to customize this for different languages without turning it off and developing the HMI to do it.

The proposal provides the INI parameter TruncateTextFieldSuffix which enables configuration of what will be appended, but by default will be set to ...

Jack-Byrne commented 3 years ago
  1. We can still do it to make it easier for English-only and other languages that support a single style of truncation, but that seems much less useful as a Core addition to me compared to providing good example truncation in the generic_hmi.

If we were implementing this in the generic hmi it would be done in the exact same way as the proposed solution, but maybe less flexible (use ellipsis only). Adding this functionality to SDL Core just makes it easier on HMI integrators as they wont have to implement the ellipsis text truncation logic.

In HTML CSS there is a property that truncates text via an ellipsis or other custom character string, so I find it hard to believe this is not an acceptable solution for SDL Core?

https://developer.mozilla.org/en-US/docs/Web/CSS/text-overflow

text-overflow: ellipsis;

We have a predefined list of languages that SDL supports so we should be able to find out which languages read RTL and appropriately set the placement of the text truncated characters and hardcode that placement in SDL Core without the need of an extra configuration.

Jack-Byrne commented 3 years ago
  1. Defining which languages are RTL might not even be neccesary. I asked @bilal-alsharifi about this and he showed me Java automagically handles this for RTL language strings. We will have to investigate if this works in C++ as well. Maybe this is built directly into the character encoding.
        String s ="م ر ح ب ا";
        s = s + "...";
        for (int i = 0; i < s.length(); i++) {
            System.out.println(s.charAt(i));
        }
م
ر
ح
ب
ا
.
.
.
joeljfischer commented 3 years ago

1. Okay, but won't this result in errors on the HMI? If an HMI declares 25 characters x 4 rows, and an app sends two lines of 30 characters each, Core will think that's fine when it's not. Again, if the app sends 30 characters, 25, 25, 25, the first line should be truncated, but instead the last line will be. What about if the app sends 3 characters, 3, 3, 3, 3 (one extra line)?

2.

The proposal provides the INI parameter TruncateTextFieldSuffix which enables configuration of what will be appended, but by default will be set to ...

My point is that this isn't helpful if multiple truncation mechanisms are required for different languages.

If we were implementing this in the generic hmi it would be done in the exact same way as the proposed solution, but maybe less flexible (use ellipsis only). Adding this functionality to SDL Core just makes it easier on HMI integrators as they wont have to implement the ellipsis text truncation logic.

The difference being that the generic_hmi is example code / a starting point and not locale-aware, whereas Core is production code and to take over truncation in this way must be locale aware.

In HTML CSS there is a property that truncates text via an ellipsis or other custom character string, so I find it hard to believe this is not an acceptable solution for SDL Core?

There's a difference between websites that are vastly more likely to be localized into one language compared to a module which is localized into dozens of languages. CSS doesn't have to be locale-aware and can provide helpers for the majority of use-cases. Creating a single-language module is not a majority use-case for SDL.

We have a predefined list of languages that SDL supports so we should be able to find out which languages read RTL and appropriately set the placement of the text truncated characters and hardcode that placement in SDL Core without the need of an extra configuration.

This is certainly an option as long as all languages that we support are okay with a single set of characters as the truncation mechanism for all languages. I did more research into how iOS deals with this (since mobile OSs are a much better analogue than websites because the OS has to support multiple languages), and it seems to only support ... out of the box, but it's hard to be certain. It does auto-change truncation for LTR vs. RTL languages. iOS offers "prefix truncation" which is ...text in LTR languages and txet... in RTL languages, "middle truncation" such as te...xt, and "suffix truncation" text... in LTR and ...txet in RTL.

I don't think we need to provide multiple truncation schemes; I'm okay with only providing suffix truncation. So, conclusion, I do think that this proposal needs to add auto-changing the truncation for known RTL languages that are SDL supported.

theresalech commented 3 years ago

The Steering Committee voted to keep this proposal in review to allow for the conversation to continue on the open items; mainly item 2, to discuss character encodings with different languages to ensure text truncation is handled correctly.

iCollin commented 3 years ago
  1. To ensure that truncation is handled correctly, we can can modify the Truncation Process section.

We could replace the statement:

If this configuration option is defined, the suffix will replace the final characters in any truncated string right up to the null terminating character.

with

If this configuration option is defined, the original string will be truncated to the maximum length allowed by the HMI minus the length of the suffix, and the suffix will be appended to the string. Core will take special care to ensure this suffix is appended correctly by checking whether the app's HMI Display Language is written as left to right or right to left.

theresalech commented 3 years ago

While it was noted that item 2 has been agreed upon (see this comment), item 1 requires additional investigation and a response from the author. The Steering Committee therefore voted to keep this proposal in review.

iCollin commented 3 years ago

1.

If an HMI declares 25 characters x 4 rows, and an app sends two lines of 30 characters each, Core will think that's fine when it's not. Again, if the app sends 30 characters, 25, 25, 25, the first line should be truncated, but instead the last line will be. What about if the app sends 3 characters, 3, 3, 3, 3 (one extra line)?

Yes these scenarios will require the HMI to do additional work before and after this proposal. This proposal doesn't intend to alleviate all TextFIeld formatting responsibility from the HMI, but just to remove some unnecessary data from being transferred to the HMI when Core can be sure it will not be used.

It will be necessary for the HMI to handle scenarios similar to the ones you describe even if Core were to truncate lines individually because not all characters are the exact same width and so Core truncating by number of characters does not guarantee a string to fit regardless. For example, this is one character: but is much wider than the average characters and the HMI's TextField widths are likely not based on this width. Since the HMI will still need to handle the fine formatting I believe the simple truncation of the whole TextField to width * rows by Core is best.

Jack-Byrne commented 3 years ago
  1. I believe that Core's scrollable message body capability is the entire length of the message, not the width of one row. I dont think the app is able to figure out how wide each row is.
        <element name="scrollableMessageBody" since="2.0">
            <description>Long form body of text that can include newlines and tabs; applies to "ScrollableMessage"</description>
        </element>

The wording in the description states new lines and tabs can be used. It does not say that the app is responsible for every line break. The HMI will handle wrapping text if a row is wider than the HMI can display.

theresalech commented 3 years ago

The Steering Committee voted to keep this proposal in review to allow for further discussion regarding Item 1.

joeljfischer commented 3 years ago

1. Ford's Sync platform at least has a text field of type ScrollableMessageBody with width=67 and rows=8

Jack-Byrne commented 3 years ago

1.

@joeljfischer My mistake, I didn't know about the width/row parameter in the rpc spec. I thought textfields only had one length parameter.

It will be necessary for the HMI to handle scenarios similar to the ones you describe even if Core were to truncate lines individually because not all characters are the exact same width and so Core truncating by number of characters does not guarantee a string to fit regardless. For example, this is one character: ﷽ but is much wider than the average characters and the HMI's TextField widths are likely not based on this width. Since the HMI will still need to handle the fine formatting I believe the simple truncation of the whole TextField to width * rows by Core is best.

I agree with Collin here. Truncating based on total length (width*row) is probably the easiest to implement and covers more cases. Using a line by line truncation technique is more complicated and could make the formatting worse in the case mentioned by Collin. Also HMI's probably have some text wrapping built in which alleviates issues that might come up specifically from scrollable message.

theresalech commented 3 years ago

The Steering Committee voted to accept this proposal with the following revision: modify "Truncation Process" section, replacing

If this configuration option is defined, the suffix will replace the final characters in any truncated string right up to the null terminating character.

with

If this configuration option is defined, the original string will be truncated to the maximum length allowed by the HMI minus the length of the suffix, and the suffix will be appended to the string. Core will take special care to ensure this suffix is appended correctly by checking whether the app's HMI Display Language is written as left to right or right to left.

theresalech commented 3 years ago

@iCollin please advise when a new PR has been entered to update the proposal to reflect the agreed upon revisions. I'll then merge the PR so the proposal is up to date, and enter an issue in the Core repository for implementation. Thanks!

theresalech commented 3 years ago

Proposal has been updated to reflect revisions, and implementation issue has been entered: https://github.com/smartdevicelink/sdl_core/issues/3716