Details

    • Type: Sub-task Sub-task
    • Status: Reopened
    • Priority: Minor Minor
    • Resolution: Unresolved
    • Affects Version/s: 2.3.1, Future Dev
    • Fix Version/s: STABLE backlog
    • Component/s: Themes
    • Labels:
    • Testing Instructions:
      Hide
      1. Add one RTL language package (Hebrew or Arabic or Farsi...) to Moodle (Home ► Site administration ► Language ► Language packs)
      2. Switch to Hebrew language (on Moodle's front page)
      3. Navigate to any page that display a TinyMCE editor ( ex. the "Description" element inside a user's profile page . OR start a new discussion on a Forum page )
      4. Click the "Add new Image" button, inside the TinyMCE editor.
      5. Validate that the Tabs you see in the pop up dialog are all left aligned (as shown in the attached image to this issue)
      6. Apply the patch
      7. Switch to Design Mode (Home ► Site administration ► Appearance ► Themes ► Theme settings)
        You might also need to set JavaScript cache to OFF. I added the following lines of code to my config.php file:
         $CFG->themedesignermode = 1; $CFG->cachejs = false; 

        Clear theme cache (Home ► Site administration ► Appearance ► Themes ► Theme selector)
        Or even:
        Purge all caches (Home ► Site administration ► Development ► Purge all caches)

      8. "Refresh" (CTRL-R / F5) the page you were viewing and make sure you see the problematic UI element... gone (Solved)
      Show
      Add one RTL language package (Hebrew or Arabic or Farsi...) to Moodle (Home ► Site administration ► Language ► Language packs) Switch to Hebrew language (on Moodle's front page) Navigate to any page that display a TinyMCE editor ( ex. the "Description" element inside a user's profile page . OR start a new discussion on a Forum page ) Click the "Add new Image" button, inside the TinyMCE editor. Validate that the Tabs you see in the pop up dialog are all left aligned (as shown in the attached image to this issue) Apply the patch Switch to Design Mode (Home ► Site administration ► Appearance ► Themes ► Theme settings) You might also need to set JavaScript cache to OFF. I added the following lines of code to my config.php file: $CFG->themedesignermode = 1; $CFG->cachejs = false ; Clear theme cache (Home ► Site administration ► Appearance ► Themes ► Theme selector) Or even: Purge all caches (Home ► Site administration ► Development ► Purge all caches) "Refresh" (CTRL-R / F5) the page you were viewing and make sure you see the problematic UI element... gone (Solved)
    • Affected Branches:
      MOODLE_23_STABLE
    • Pull from Repository:
    • Pull Master Branch:
      WIP-MDL-35250-master
    • Rank:
      43900

      Description

      Right align all TABs in all TinyMCE dialogs, when in RTL mode

      Insert Image dialog
      Insert Link dialog
      Insert Multimedia dialog

        Issue Links

          Activity

          Hide
          Nadav Kavalerchik added a comment - - edited

          TEXTAREA fields inside TinyMCE setting page should be always left aligned. in RTL and LTR modes. (Since button names are always in English)

          proposed fix: https://github.com/nadavkav/moodle/commit/344be17b4e21f7547bf58b8d175f87225ed40ddd

          Show
          Nadav Kavalerchik added a comment - - edited TEXTAREA fields inside TinyMCE setting page should be always left aligned. in RTL and LTR modes. (Since button names are always in English) proposed fix: https://github.com/nadavkav/moodle/commit/344be17b4e21f7547bf58b8d175f87225ed40ddd
          Hide
          Nadav Kavalerchik added a comment -

          @Mary

          I would really like to move all the changes I made to the:
          lib/editor/tinymce/tiny_mce/3.6.0/themes/advanced/skins/o2k7/dialog.css
          (https://github.com/nadavkav/moodle/commit/48dbb9aa8b4165c37f1b19ae9d99ee81c83d8b4b)
          into the theme/base style files.
          BUT, have no idea how to. Can you help?

          Show
          Nadav Kavalerchik added a comment - @Mary I would really like to move all the changes I made to the: lib/editor/tinymce/tiny_mce/3.6.0/themes/advanced/skins/o2k7/dialog.css ( https://github.com/nadavkav/moodle/commit/48dbb9aa8b4165c37f1b19ae9d99ee81c83d8b4b ) into the theme/base style files. BUT, have no idea how to. Can you help?
          Hide
          Mary Evans added a comment -

          @Nadav

          If the changes are to style the TinyMCE editor then they are better left where they are rather than add them to base theme.

          Do they do the job of styling the elements of the editor when in RTL? If the answer is yes then leave them there.

          Show
          Mary Evans added a comment - @Nadav If the changes are to style the TinyMCE editor then they are better left where they are rather than add them to base theme. Do they do the job of styling the elements of the editor when in RTL? If the answer is yes then leave them there.
          Hide
          Nadav Kavalerchik added a comment -

          Yes, it does solve the issue but...
          The thing is...
          Each time Moodle pull an upstream version of TinyMCE we (I) need to move the changes (fixes) to the new version's files.
          I am sure this not the way to go.

          Show
          Nadav Kavalerchik added a comment - Yes, it does solve the issue but... The thing is... Each time Moodle pull an upstream version of TinyMCE we (I) need to move the changes (fixes) to the new version's files. I am sure this not the way to go.
          Hide
          Petr Škoda added a comment -

          Hi, yes, imported TinyMCE is not going to be modified at all in future moodle releases, we need to find a way to keep the CSS hacks in a separate location or better submit them upstream. Did you try to use TinyMCE forums?

          Show
          Petr Škoda added a comment - Hi, yes, imported TinyMCE is not going to be modified at all in future moodle releases, we need to find a way to keep the CSS hacks in a separate location or better submit them upstream. Did you try to use TinyMCE forums?
          Hide
          Nadav Kavalerchik added a comment -

          @Petr

          I have not tried posting the fixes to upstream TinyMCE since I thought we could somehow cascade the CSS fixes over their skins. I will.

          BTW, I (we) have similar issues with YUI3 : MDL-35267 (What's your take on that?)

          Show
          Nadav Kavalerchik added a comment - @Petr I have not tried posting the fixes to upstream TinyMCE since I thought we could somehow cascade the CSS fixes over their skins. I will. BTW, I (we) have similar issues with YUI3 : MDL-35267 (What's your take on that?)
          Hide
          Nadav Kavalerchik added a comment -

          Reported Upstream, on TinyMCE's tracker : http://www.tinymce.com/develop/bugtracker_view.php?id=5509

          Show
          Nadav Kavalerchik added a comment - Reported Upstream, on TinyMCE's tracker : http://www.tinymce.com/develop/bugtracker_view.php?id=5509
          Hide
          Dan Poltawski added a comment -

          Sending all 'waiting for peer review' issues to integration before freeze, as agreed in Integrators Meeting 19/10/12. We are doing this to ensure any 'integratable issues' will not got missed before freeze..

          Show
          Dan Poltawski added a comment - Sending all 'waiting for peer review' issues to integration before freeze, as agreed in Integrators Meeting 19/10/12. We are doing this to ensure any 'integratable issues' will not got missed before freeze..
          Hide
          Nadav Kavalerchik added a comment -

          Bless you Dan and Moodle HQ team

          Show
          Nadav Kavalerchik added a comment - Bless you Dan and Moodle HQ team
          Hide
          Dan Poltawski added a comment -

          The main moodle.git repository has just been updated with latest weekly modifications. You may wish to rebase your PULL branches to simplify history and avoid any possible merge conflicts. This would also make integrator's life easier next week.

          TIA and ciao

          Show
          Dan Poltawski added a comment - The main moodle.git repository has just been updated with latest weekly modifications. You may wish to rebase your PULL branches to simplify history and avoid any possible merge conflicts. This would also make integrator's life easier next week. TIA and ciao
          Hide
          Nadav Kavalerchik added a comment -

          Rebased over latest 1-11-2012 master
          Ready for peer review

          Show
          Nadav Kavalerchik added a comment - Rebased over latest 1-11-2012 master Ready for peer review
          Hide
          Sam Hemelryk added a comment -

          Hi Nadav,

          Petr is correct in noting that we shouldn't modify anything within TinyMCE.
          In this case there is an alternative as well I think, theme/base/styles/editor.css is loaded into the editor only. If you add your styles there everything should work and we'd have a solution that could be integrated.

          Many thanks
          Sam

          Show
          Sam Hemelryk added a comment - Hi Nadav, Petr is correct in noting that we shouldn't modify anything within TinyMCE. In this case there is an alternative as well I think, theme/base/styles/editor.css is loaded into the editor only. If you add your styles there everything should work and we'd have a solution that could be integrated. Many thanks Sam
          Hide
          CiBoT added a comment -

          Moving this reopened issue out from current integration. Please, re-submit it for integration once ready.

          Show
          CiBoT added a comment - Moving this reopened issue out from current integration. Please, re-submit it for integration once ready.
          Hide
          Nadav Kavalerchik added a comment -

          Sam,

          I tried that in the past, and it did not help since that specific file is not included in the popup HEAD tag

          <head>
              <meta http-equiv="content-type" content="text/html; charset=utf-8">
              <title>הוספה/עריכת תמונה</title>
              </script><script type="text/javascript" src="http://localhost/moodle-org/master/lib/editor/tinymce/tiny_mce/3.5.7/tiny_mce_popup.js"></script>
              <link rel="stylesheet" href="http://localhost/moodle-org/master/lib/editor/tinymce/tiny_mce/3.5.7/themes/advanced/skins/o2k7/dialog.css">
              <script type="text/javascript" src="http://localhost/moodle-org/master/lib/editor/tinymce/tiny_mce/3.5.7/utils/mctabs.js"></script>
              <style type="text/css"></style>
              <script type="text/javascript" src="http://localhost/moodle-org/master/lib/editor/tinymce/tiny_mce/3.5.7/utils/validate.js"></script>
              <script type="text/javascript" src="http://localhost/moodle-org/master/lib/editor/tinymce/tiny_mce/3.5.7/utils/form_utils.js"></script>
              <script type="text/javascript" src="http://localhost/moodle-org/master/lib/editor/tinymce/tiny_mce/3.5.7/utils/editable_selects.js"></script>
              <script type="text/javascript" src="js/image.js"></script>
              <link href="css/image.css" rel="stylesheet" type="text/css">
          </head>
          

          lib/editor/tinymce/tiny_mce/3.5.7/themes/advanced/skins/o2k7/dialog.css is the only CSS file i can work with

          Any suggestions?

          Show
          Nadav Kavalerchik added a comment - Sam, I tried that in the past, and it did not help since that specific file is not included in the popup HEAD tag <head> <meta http-equiv= "content-type" content= "text/html; charset=utf-8" > <title>הוספה/עריכת תמונה</title> </script><script type= "text/javascript" src= "http: //localhost/moodle-org/master/lib/editor/tinymce/tiny_mce/3.5.7/tiny_mce_popup.js" ></script> <link rel= "stylesheet" href= "http: //localhost/moodle-org/master/lib/editor/tinymce/tiny_mce/3.5.7/themes/advanced/skins/o2k7/dialog.css" > <script type= "text/javascript" src= "http: //localhost/moodle-org/master/lib/editor/tinymce/tiny_mce/3.5.7/utils/mctabs.js" ></script> <style type= "text/css" ></style> <script type= "text/javascript" src= "http: //localhost/moodle-org/master/lib/editor/tinymce/tiny_mce/3.5.7/utils/validate.js" ></script> <script type= "text/javascript" src= "http: //localhost/moodle-org/master/lib/editor/tinymce/tiny_mce/3.5.7/utils/form_utils.js" ></script> <script type= "text/javascript" src= "http: //localhost/moodle-org/master/lib/editor/tinymce/tiny_mce/3.5.7/utils/editable_selects.js" ></script> <script type= "text/javascript" src= "js/image.js" ></script> <link href= "css/image.css" rel= "stylesheet" type= "text/css" > </head> lib/editor/tinymce/tiny_mce/3.5.7/themes/advanced/skins/o2k7/dialog.css is the only CSS file i can work with Any suggestions?
          Hide
          Rajesh Taneja added a comment -

          Hello Nadav,

          You probably have to look at individual dialogue and update css to support rtl.
          eg: For moodleimage: Moodle/lib/editor/tinymce/plugins/moodleimage/tinymce/css/image.css needs to be updated.

          But the problem is iframe dialog html doesn't have rtl information, so this needs to be fixed in code.
          Will talk to Sam and see what is the best way to support this.

          Show
          Rajesh Taneja added a comment - Hello Nadav, You probably have to look at individual dialogue and update css to support rtl. eg: For moodleimage: Moodle/lib/editor/tinymce/plugins/moodleimage/tinymce/css/image.css needs to be updated. But the problem is iframe dialog html doesn't have rtl information, so this needs to be fixed in code. Will talk to Sam and see what is the best way to support this.
          Hide
          Nadav Kavalerchik added a comment -

          Rajesh, Thank you for your suggestion. Though it only solved the "Image popup dialog" and I am not sure how to fix the: "Find and Replace", "Insert Table" and "More colors" dialogs.

          I have updated the code in the branch.
          ( removed it from: lib/editor/tinymce/tiny_mce/3.5.7/themes/advanced/skins/o2k7/dialog.css and added it into: /lib/editor/tinymce/plugins/moodleimage/tinymce/css/image.css)

          Show
          Nadav Kavalerchik added a comment - Rajesh, Thank you for your suggestion. Though it only solved the "Image popup dialog" and I am not sure how to fix the: "Find and Replace", "Insert Table" and "More colors" dialogs. I have updated the code in the branch. ( removed it from: lib/editor/tinymce/tiny_mce/3.5.7/themes/advanced/skins/o2k7/dialog.css and added it into: /lib/editor/tinymce/plugins/moodleimage/tinymce/css/image.css)
          Hide
          Aparup Banerjee added a comment -

          Hi Nadav, just wondering did you want to update the status of this? its in 'reopened' atm and i'm not sure if the remaining issues could be fixed in a separate issue.. (haven't looked at this MDL much)

          Show
          Aparup Banerjee added a comment - Hi Nadav, just wondering did you want to update the status of this? its in 'reopened' atm and i'm not sure if the remaining issues could be fixed in a separate issue.. (haven't looked at this MDL much)
          Hide
          Nadav Kavalerchik added a comment -

          @Aparup,
          Currently, after updating the patch with Rajesh suggestions, this issue is not completely solved.

          I am waiting for an answer from any of the Moodle HQ team for how to solve this issue completely, on all dialogs not just the "Insert Image" dialog (As suggested by Rajesh)

          Show
          Nadav Kavalerchik added a comment - @Aparup, Currently, after updating the patch with Rajesh suggestions, this issue is not completely solved. I am waiting for an answer from any of the Moodle HQ team for how to solve this issue completely, on all dialogs not just the "Insert Image" dialog (As suggested by Rajesh)
          Hide
          Rajesh Taneja added a comment -

          Sorry Nadav,

          I have been trying to get a good solution for this. The problem is html in dialogue doesn't have rtl information and that is bottleneck.

          Had a word with Sam and there are two things which you can use for now:

          1. /lib/editor/tinymce/lib.php line - 144 add
            'popup_css_add' => $contentcss,
            

            This will allow editor.css to be added to dialogue pop-up

          2. If these changes are specific to tinymce then add your css to /lib/editor/tinymce/editor_styles.css else if this is for all supported editors add changes to /theme/base/style/editor.css

          Although this will not solve the issue yet. I am still trying to think of a way to add rtl information in dialogue iframe, so we can apply the correct css.

          Show
          Rajesh Taneja added a comment - Sorry Nadav, I have been trying to get a good solution for this. The problem is html in dialogue doesn't have rtl information and that is bottleneck. Had a word with Sam and there are two things which you can use for now: /lib/editor/tinymce/lib.php line - 144 add 'popup_css_add' => $contentcss, This will allow editor.css to be added to dialogue pop-up If these changes are specific to tinymce then add your css to /lib/editor/tinymce/editor_styles.css else if this is for all supported editors add changes to /theme/base/style/editor.css Although this will not solve the issue yet. I am still trying to think of a way to add rtl information in dialogue iframe, so we can apply the correct css.
          Show
          Rajesh Taneja added a comment - If we can add Sam's patch, https://github.com/samhemelryk/tinymce/compare/master...directionality Then following should solve this issue. https://github.com/rajeshtaneja/moodle/compare/master...wip-mdl-35250
          Hide
          Nadav Kavalerchik added a comment -

          +1 from me (to Sam's patch and Rajesh solution)

          Rajesh, please see that you go through all the popups in the TinyMCE editor and all are right aligned. just in case...
          (Since, I think I had more CSS rules in the original code, which cover more cases)

          Show
          Nadav Kavalerchik added a comment - +1 from me (to Sam's patch and Rajesh solution) Rajesh, please see that you go through all the popups in the TinyMCE editor and all are right aligned. just in case... (Since, I think I had more CSS rules in the original code, which cover more cases)
          Hide
          Rajesh Taneja added a comment -

          Thanks Nadav,

          You had more css and I tried consolidate them. Will check again, in case I missed something.

          Show
          Rajesh Taneja added a comment - Thanks Nadav, You had more css and I tried consolidate them. Will check again, in case I missed something.
          Hide
          Sam Hemelryk added a comment -

          Hi just,

          Just noting I've talked to Petr about this issue as he's worked with TinyMCE closely and has provided upstream patches at the moment.
          He's said in the past they've been rather slow in getting things into core and that patching TinyMCE is not going to be ideal, especially as this patch modifies popup.js which is one long line (means every time we import we are going to hit conflicts).
          We decided the best solution will be to monkey patch the code (patch the runtime code) which has been done before through the tinymce module.js file.
          I'll work on a patch this afternoon for that and then we can look to get something up to allow these changes.

          Many thanks
          Sam

          Show
          Sam Hemelryk added a comment - Hi just, Just noting I've talked to Petr about this issue as he's worked with TinyMCE closely and has provided upstream patches at the moment. He's said in the past they've been rather slow in getting things into core and that patching TinyMCE is not going to be ideal, especially as this patch modifies popup.js which is one long line (means every time we import we are going to hit conflicts). We decided the best solution will be to monkey patch the code (patch the runtime code) which has been done before through the tinymce module.js file. I'll work on a patch this afternoon for that and then we can look to get something up to allow these changes. Many thanks Sam
          Hide
          Nadav Kavalerchik added a comment -

          Sam Hemelryk,
          Any news on integrating Rajesh Taneja fix (two comment above)?

          Show
          Nadav Kavalerchik added a comment - Sam Hemelryk , Any news on integrating Rajesh Taneja fix (two comment above)?
          Hide
          Sam Hemelryk added a comment -

          Hi Nadav,

          My fix wasn't accepted upstream as they now only accept critial bug fixes to the 3.x branch of TinyMCE. https://github.com/tinymce/tinymce/pull/162
          This means presently that Raj's fix won't work. We'll need to create a monkey patch now in order to apply the required change before accepting Raj's fix.

          Cheers
          Sam

          Show
          Sam Hemelryk added a comment - Hi Nadav, My fix wasn't accepted upstream as they now only accept critial bug fixes to the 3.x branch of TinyMCE. https://github.com/tinymce/tinymce/pull/162 This means presently that Raj's fix won't work. We'll need to create a monkey patch now in order to apply the required change before accepting Raj's fix. Cheers Sam
          Hide
          Nadav Kavalerchik added a comment -

          Hi Sam,

          Thanks you for the update. I left them a comment to reconsider it. Hope it helps.
          If not, please consider sending a new pull request for 4.x branch. If it is ok with you.

          Thank you for helping pushing this forward!
          Nadav

          Show
          Nadav Kavalerchik added a comment - Hi Sam, Thanks you for the update. I left them a comment to reconsider it. Hope it helps. If not, please consider sending a new pull request for 4.x branch. If it is ok with you. Thank you for helping pushing this forward! Nadav

            People

            • Votes:
              1 Vote for this issue
              Watchers:
              5 Start watching this issue

              Dates

              • Created:
                Updated: