Change RTL detection to rely on unicode-bidi paragraph by paragraph (#…

…14573)
mastodon · Dec 15, 2020 · 1f56405 · 1f56405
1 parent 1045549
commit 1f56405
Show file tree

Hide file tree

Showing 12 changed files with 26 additions and 106 deletions.
diff --git a/app/helpers/statuses_helper.rb b/app/helpers/statuses_helper.rb
@@ -92,22 +92,6 @@ def microformats_h_class(status, is_predecessor, is_successor, include_threads)
     end
   end
 
-  def rtl_status?(status)
-    status.local? ? rtl?(status.text) : rtl?(strip_tags(status.text))
-  end
-
-  def rtl?(text)
-    text = simplified_text(text)
-    rtl_words = text.scan(/[\p{Hebrew}\p{Arabic}\p{Syriac}\p{Thaana}\p{Nko}]+/m)
-
-    if rtl_words.present?
-      total_size = text.size.to_f
-      rtl_size(rtl_words) / total_size > 0.3
-    else
-      false
-    end
-  end
-
   def fa_visibility_icon(status)
     case status.visibility
     when 'public'
@@ -143,10 +127,6 @@ def simplified_text(text)
     end
   end
 
-  def rtl_size(words)
-    words.reduce(0) { |acc, elem| acc + elem.size }.to_f
-  end
-
   def embedded_view?
     params[:controller] == EMBEDDED_CONTROLLER && params[:action] == EMBEDDED_ACTION
   end

diff --git a/app/javascript/mastodon/components/autosuggest_input.js b/app/javascript/mastodon/components/autosuggest_input.js
@@ -4,7 +4,6 @@ import AutosuggestEmoji from './autosuggest_emoji';
 import AutosuggestHashtag from './autosuggest_hashtag';
 import ImmutablePropTypes from 'react-immutable-proptypes';
 import PropTypes from 'prop-types';
-import { isRtl } from '../rtl';
 import ImmutablePureComponent from 'react-immutable-pure-component';
 import classNames from 'classnames';
 import { List as ImmutableList } from 'immutable';
@@ -189,11 +188,6 @@ export default class AutosuggestInput extends ImmutablePureComponent {
   render () {
     const { value, suggestions, disabled, placeholder, onKeyUp, autoFocus, className, id, maxLength } = this.props;
     const { suggestionsHidden } = this.state;
-    const style = { direction: 'ltr' };
-
-    if (isRtl(value)) {
-      style.direction = 'rtl';
-    }
 
     return (
       <div className='autosuggest-input'>
@@ -212,7 +206,7 @@ export default class AutosuggestInput extends ImmutablePureComponent {
             onKeyUp={onKeyUp}
             onFocus={this.onFocus}
             onBlur={this.onBlur}
-            style={style}
+            dir='auto'
             aria-autocomplete='list'
             id={id}
             className={className}

diff --git a/app/javascript/mastodon/components/autosuggest_textarea.js b/app/javascript/mastodon/components/autosuggest_textarea.js
@@ -4,7 +4,6 @@ import AutosuggestEmoji from './autosuggest_emoji';
 import AutosuggestHashtag from './autosuggest_hashtag';
 import ImmutablePropTypes from 'react-immutable-proptypes';
 import PropTypes from 'prop-types';
-import { isRtl } from '../rtl';
 import ImmutablePureComponent from 'react-immutable-pure-component';
 import Textarea from 'react-textarea-autosize';
 import classNames from 'classnames';
@@ -195,11 +194,6 @@ export default class AutosuggestTextarea extends ImmutablePureComponent {
   render () {
     const { value, suggestions, disabled, placeholder, onKeyUp, autoFocus, children } = this.props;
     const { suggestionsHidden } = this.state;
-    const style = { direction: 'ltr' };
-
-    if (isRtl(value)) {
-      style.direction = 'rtl';
-    }
 
     return [
       <div className='compose-form__autosuggest-wrapper' key='autosuggest-wrapper'>
@@ -220,7 +214,7 @@ export default class AutosuggestTextarea extends ImmutablePureComponent {
               onFocus={this.onFocus}
               onBlur={this.onBlur}
               onPaste={this.onPaste}
-              style={style}
+              dir='auto'
               aria-autocomplete='list'
             />
           </label>

diff --git a/app/javascript/mastodon/components/status_content.js b/app/javascript/mastodon/components/status_content.js
@@ -1,7 +1,6 @@
 import React from 'react';
 import ImmutablePropTypes from 'react-immutable-proptypes';
 import PropTypes from 'prop-types';
-import { isRtl } from '../rtl';
 import { FormattedMessage } from 'react-intl';
 import Permalink from './permalink';
 import classnames from 'classnames';
@@ -186,17 +185,12 @@ export default class StatusContent extends React.PureComponent {
 
     const content = { __html: status.get('contentHtml') };
     const spoilerContent = { __html: status.get('spoilerHtml') };
-    const directionStyle = { direction: 'ltr' };
     const classNames = classnames('status__content', {
       'status__content--with-action': this.props.onClick && this.context.router,
       'status__content--with-spoiler': status.get('spoiler_text').length > 0,
       'status__content--collapsed': renderReadMore,
     });
 
-    if (isRtl(status.get('search_index'))) {
-      directionStyle.direction = 'rtl';
-    }
-
     const showThreadButton = (
       <button className='status__content__read-more-button' onClick={this.props.onClick}>
         <FormattedMessage id='status.show_thread' defaultMessage='Show thread' />
@@ -225,7 +219,7 @@ export default class StatusContent extends React.PureComponent {
       }
 
       return (
-        <div className={classNames} ref={this.setRef} tabIndex='0' style={directionStyle} onMouseDown={this.handleMouseDown} onMouseUp={this.handleMouseUp}>
+        <div className={classNames} ref={this.setRef} tabIndex='0' onMouseDown={this.handleMouseDown} onMouseUp={this.handleMouseUp}>
           <p style={{ marginBottom: hidden && status.get('mentions').isEmpty() ? '0px' : null }}>
             <span dangerouslySetInnerHTML={spoilerContent} />
             {' '}
@@ -234,7 +228,7 @@ export default class StatusContent extends React.PureComponent {
 
           {mentionsPlaceholder}
 
-          <div tabIndex={!hidden ? 0 : null} className={`status__content__text ${!hidden ? 'status__content__text--visible' : ''}`} style={directionStyle} dangerouslySetInnerHTML={content} />
+          <div tabIndex={!hidden ? 0 : null} className={`status__content__text ${!hidden ? 'status__content__text--visible' : ''}`} dangerouslySetInnerHTML={content} />
 
           {!hidden && !!status.get('poll') && <PollContainer pollId={status.get('poll')} />}
 
@@ -243,8 +237,8 @@ export default class StatusContent extends React.PureComponent {
       );
     } else if (this.props.onClick) {
       const output = [
-        <div className={classNames} ref={this.setRef} tabIndex='0' style={directionStyle} onMouseDown={this.handleMouseDown} onMouseUp={this.handleMouseUp} key='status-content'>
-          <div className='status__content__text status__content__text--visible' style={directionStyle} dangerouslySetInnerHTML={content} />
+        <div className={classNames} ref={this.setRef} tabIndex='0' onMouseDown={this.handleMouseDown} onMouseUp={this.handleMouseUp} key='status-content'>
+          <div className='status__content__text status__content__text--visible' dangerouslySetInnerHTML={content} />
 
           {!!status.get('poll') && <PollContainer pollId={status.get('poll')} />}
 
@@ -259,8 +253,8 @@ export default class StatusContent extends React.PureComponent {
       return output;
     } else {
       return (
-        <div className={classNames} ref={this.setRef} tabIndex='0' style={directionStyle}>
-          <div className='status__content__text status__content__text--visible' style={directionStyle} dangerouslySetInnerHTML={content} />
+        <div className={classNames} ref={this.setRef} tabIndex='0'>
+          <div className='status__content__text status__content__text--visible' dangerouslySetInnerHTML={content} />
 
           {!!status.get('poll') && <PollContainer pollId={status.get('poll')} />}
 

diff --git a/app/javascript/mastodon/features/compose/components/reply_indicator.js b/app/javascript/mastodon/features/compose/components/reply_indicator.js
@@ -6,7 +6,6 @@ import IconButton from '../../../components/icon_button';
 import DisplayName from '../../../components/display_name';
 import { defineMessages, injectIntl } from 'react-intl';
 import ImmutablePureComponent from 'react-immutable-pure-component';
-import { isRtl } from '../../../rtl';
 import AttachmentList from 'mastodon/components/attachment_list';
 
 const messages = defineMessages({
@@ -45,9 +44,6 @@ class ReplyIndicator extends ImmutablePureComponent {
     }
 
     const content = { __html: status.get('contentHtml') };
-    const style   = {
-      direction: isRtl(status.get('search_index')) ? 'rtl' : 'ltr',
-    };
 
     return (
       <div className='reply-indicator'>
@@ -60,7 +56,7 @@ class ReplyIndicator extends ImmutablePureComponent {
           </a>
         </div>
 
-        <div className='reply-indicator__content' style={style} dangerouslySetInnerHTML={content} />
+        <div className='reply-indicator__content' dangerouslySetInnerHTML={content} />
 
         {status.get('media_attachments').size > 0 && (
           <AttachmentList

diff --git a/app/javascript/mastodon/rtl.js b/app/javascript/mastodon/rtl.js
diff --git a/app/javascript/styles/mailer.scss b/app/javascript/styles/mailer.scss
@@ -58,6 +58,16 @@ td {
   vertical-align: top;
 }
 
+.auto-dir {
+  p {
+    unicode-bidi: plaintext;
+  }
+
+  a {
+    unicode-bidi: isolate;
+  }
+}
+
 .email-table,
 .content-section,
 .column,
@@ -96,7 +106,7 @@ body {
 .col-3,
 .col-4,
 .col-5,
-.col-6, {
+.col-6 {
   font-size: 0;
   display: inline-block;
   width: 100%;

diff --git a/app/javascript/styles/mastodon/components.scss b/app/javascript/styles/mastodon/components.scss
@@ -831,6 +831,7 @@
   p {
     margin-bottom: 20px;
     white-space: pre-wrap;
+    unicode-bidi: plaintext;
 
     &:last-child {
       margin-bottom: 0;
@@ -840,6 +841,7 @@
   a {
     color: $secondary-text-color;
     text-decoration: none;
+    unicode-bidi: isolate;
 
     &:hover {
       text-decoration: underline;

diff --git a/app/views/notification_mailer/_status.html.haml b/app/views/notification_mailer/_status.html.haml
@@ -26,11 +26,11 @@
                                       = "@#{status.account.acct}"
 
                               - if status.spoiler_text?
-                                %div{ dir: rtl_status?(status) ? 'rtl' : 'ltr' }
+                                %div.auto-dir
                                   %p
                                     = Formatter.instance.format_spoiler(status)
 
-                              %div{ dir: rtl_status?(status) ? 'rtl' : 'ltr' }
+                              %div.auto-dir
                                 = Formatter.instance.format(status)
 
                                 - if status.media_attachments.size > 0

diff --git a/app/views/statuses/_detailed_status.html.haml b/app/views/statuses/_detailed_status.html.haml
@@ -20,7 +20,7 @@
       %p<
         %span.p-summary> #{Formatter.instance.format_spoiler(status, autoplay: autoplay)}&nbsp;
         %button.status__content__spoiler-link= t('statuses.show_more')
-    .e-content{ dir: rtl_status?(status) ? 'rtl' : 'ltr' }
+    .e-content
       = Formatter.instance.format(status, custom_emojify: true, autoplay: autoplay)
       - if status.preloadable_poll
         = react_component :poll, disabled: true, poll: ActiveModelSerializers::SerializableResource.new(status.preloadable_poll, serializer: REST::PollSerializer, scope: current_user, scope_name: :current_user).as_json do

diff --git a/app/views/statuses/_simple_status.html.haml b/app/views/statuses/_simple_status.html.haml
@@ -29,7 +29,7 @@
       %p<
         %span.p-summary> #{Formatter.instance.format_spoiler(status, autoplay: autoplay)}&nbsp;
         %button.status__content__spoiler-link= t('statuses.show_more')
-    .e-content{ dir: rtl_status?(status) ? 'rtl' : 'ltr' }
+    .e-content
       = Formatter.instance.format(status, custom_emojify: true, autoplay: autoplay)
       - if status.preloadable_poll
         = react_component :poll, disabled: true, poll: ActiveModelSerializers::SerializableResource.new(status.preloadable_poll, serializer: REST::PollSerializer, scope: current_user, scope_name: :current_user).as_json do

diff --git a/spec/helpers/statuses_helper_spec.rb b/spec/helpers/statuses_helper_spec.rb
@@ -149,22 +149,4 @@ def set_embedded_view
       expect(css_class).to eq 'h-cite'
     end
   end
-
-  describe '#rtl?' do
-    it 'is false if text is empty' do
-      expect(helper).not_to be_rtl ''
-    end
-
-    it 'is false if there are no right to left characters' do
-      expect(helper).not_to be_rtl 'hello world'
-    end
-
-    it 'is false if right to left characters are fewer than 1/3 of total text' do
-      expect(helper).not_to be_rtl 'hello ݟ world'
-    end
-
-    it 'is true if right to left characters are greater than 1/3 of total text' do
-      expect(helper).to be_rtl 'aaݟaaݟ'
-    end
-  end
 end