Details

    • Type: Sub-task
    • Status: Reopened
    • Priority: 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

      Description

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

      Insert Image dialog
      Insert Link dialog
      Insert Multimedia dialog

        Gliffy Diagrams

          Issue Links

            Activity

            Hide
            nadavkav 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
            nadavkav 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
            nadavkav 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
            nadavkav 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
            lazydaisy 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
            lazydaisy 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
            nadavkav 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
            nadavkav 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
            skodak Petr Skoda 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
            skodak Petr Skoda 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
            nadavkav 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
            nadavkav 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
            nadavkav Nadav Kavalerchik added a comment -

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

            Show
            nadavkav Nadav Kavalerchik added a comment - Reported Upstream, on TinyMCE's tracker : http://www.tinymce.com/develop/bugtracker_view.php?id=5509
            Hide
            poltawski 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
            poltawski 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
            nadavkav Nadav Kavalerchik added a comment -

            Bless you Dan and Moodle HQ team

            Show
            nadavkav Nadav Kavalerchik added a comment - Bless you Dan and Moodle HQ team
            Hide
            poltawski 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
            poltawski 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
            nadavkav Nadav Kavalerchik added a comment -

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

            Show
            nadavkav Nadav Kavalerchik added a comment - Rebased over latest 1-11-2012 master Ready for peer review
            Hide
            samhemelryk 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
            samhemelryk 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 CiBoT added a comment -

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

            Show
            cibot CiBoT added a comment - Moving this reopened issue out from current integration. Please, re-submit it for integration once ready.
            Hide
            nadavkav 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
            nadavkav 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
            rajeshtaneja 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
            rajeshtaneja 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
            nadavkav 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
            nadavkav 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
            nebgor 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
            nebgor 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
            nadavkav 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
            nadavkav 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
            rajeshtaneja 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
            rajeshtaneja 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
            rajeshtaneja 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
            nadavkav 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
            nadavkav 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
            rajeshtaneja 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
            rajeshtaneja 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
            samhemelryk 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
            samhemelryk 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
            nadavkav Nadav Kavalerchik added a comment -

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

            Show
            nadavkav Nadav Kavalerchik added a comment - Sam Hemelryk , Any news on integrating Rajesh Taneja fix (two comment above)?
            Hide
            samhemelryk 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
            samhemelryk 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
            nadavkav 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
            nadavkav 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: