Details

    • Type: Sub-task
    • Status: Closed
    • Priority: Blocker
    • Resolution: Fixed
    • Affects Version/s: 2.0.1
    • Fix Version/s: 2.0.4
    • Component/s: Wiki (2.x)
    • Testing Instructions:
      Hide

      To test:
      1. Adding a new file using 'Files' tab in wiki
      2. Create a page using nwiki or creole format
      3. Select the file you uploaded in step 1
      4. save the page, you will be able to see the image in wik

      Please note:
      You need to test it using Collaborative wiki and Individual Wiki with group mode on/off.

      Thanks.

      Show
      To test: 1. Adding a new file using 'Files' tab in wiki 2. Create a page using nwiki or creole format 3. Select the file you uploaded in step 1 4. save the page, you will be able to see the image in wik Please note: You need to test it using Collaborative wiki and Individual Wiki with group mode on/off. Thanks.
    • Affected Branches:
      MOODLE_20_STABLE
    • Fixed Branches:
      MOODLE_20_STABLE
    • Pull Master Branch:
      s9_MDL-26392_wiki_files_master

      Description

      As reported by Gisela Hillenbrand in http://moodle.org/mod/forum/discuss.php?d=168577

      I use the NWiki format. Clicking on the image icon in the wiki page edit mode results in the piece of code: [[image:Image|alt]] - so where can I upload a image.jpg file and link it with that Image in my wiki?

        Gliffy Diagrams

          Issue Links

            Activity

            tsala Helen Foster created issue -
            tsala Helen Foster made changes -
            Field Original Value New Value
            Link This issue will be resolved by MDL-20814 [ MDL-20814 ]
            dougiamas Martin Dougiamas made changes -
            Fix Version/s STABLE Sprint 6 [ 10535 ]
            Fix Version/s STABLE backlog [ 10463 ]
            dongsheng Dongsheng Cai made changes -
            Assignee moodle.com [ moodle.com ] Dongsheng Cai [ dongsheng ]
            dongsheng Dongsheng Cai made changes -
            Status Open [ 1 ] In Progress [ 3 ]
            Hide
            dongsheng Dongsheng Cai added a comment -

            Helen, we never support images for nwiki format, this is more like a new feature, not a blocker, I'm not sure if we should implement this in this sprint, I will check with Martin.

            Show
            dongsheng Dongsheng Cai added a comment - Helen, we never support images for nwiki format, this is more like a new feature, not a blocker, I'm not sure if we should implement this in this sprint, I will check with Martin.
            Hide
            dougiamas Martin Dougiamas added a comment -

            Can't be added in an existing sprint ...

            Show
            dougiamas Martin Dougiamas added a comment - Can't be added in an existing sprint ...
            dougiamas Martin Dougiamas made changes -
            Fix Version/s STABLE backlog [ 10463 ]
            Fix Version/s STABLE Sprint 6 [ 10535 ]
            Hide
            dougiamas Martin Dougiamas added a comment -

            Sorry, I was confused. It was in the sprint already

            I guess image support is a whole new feature, yes. But the fact that the bogus image button is there is a bug, so that should be fixed.

            Show
            dougiamas Martin Dougiamas added a comment - Sorry, I was confused. It was in the sprint already I guess image support is a whole new feature, yes. But the fact that the bogus image button is there is a bug, so that should be fixed.
            dougiamas Martin Dougiamas made changes -
            Fix Version/s STABLE Sprint 6 [ 10535 ]
            Fix Version/s STABLE backlog [ 10463 ]
            dongsheng Dongsheng Cai made changes -
            Status In Progress [ 3 ] Open [ 1 ]
            Hide
            dongsheng Dongsheng Cai added a comment -

            Martin, the image button is not completely not working, it is just not working with the new file system, if you put a image url there (I means external image file), it will be displayed.

            Show
            dongsheng Dongsheng Cai added a comment - Martin, the image button is not completely not working, it is just not working with the new file system, if you put a image url there (I means external image file), it will be displayed.
            Hide
            dougiamas Martin Dougiamas added a comment -

            OK, so after discussion I think it's worth it to do this in stable:

            1) Add a filemanager form element to the nwiki editing form so that images (only, for now) can be uploaded
            2) Make sure the nwiki parser can securely convert [image:somefile.jpg] into a HTML img tag via pluginfile.php

            The larger problem about other file types needs to be thought about some other time. Potential here for people to start adding huge file collections and perhaps avoid quotas etc.

            Show
            dougiamas Martin Dougiamas added a comment - OK, so after discussion I think it's worth it to do this in stable: 1) Add a filemanager form element to the nwiki editing form so that images (only, for now) can be uploaded 2) Make sure the nwiki parser can securely convert [image:somefile.jpg] into a HTML img tag via pluginfile.php The larger problem about other file types needs to be thought about some other time. Potential here for people to start adding huge file collections and perhaps avoid quotas etc.
            mudrd8mz David Mudrak made changes -
            Link This issue has a non-specific relationship to MDL-25629 [ MDL-25629 ]
            Hide
            mudrd8mz David Mudrak added a comment -

            I just added a link to MDL-25629 that is kind of similar issue, affecting all Moodle (not just single module).

            Show
            mudrd8mz David Mudrak added a comment - I just added a link to MDL-25629 that is kind of similar issue, affecting all Moodle (not just single module).
            dongsheng Dongsheng Cai made changes -
            Status Open [ 1 ] In Progress [ 3 ]
            Hide
            dongsheng Dongsheng Cai added a comment -
            Show
            dongsheng Dongsheng Cai added a comment - Adding Aparup here for peer review: https://github.com/dongsheng/moodle/commit/119e7d4884bfc13ef234280a8e18764b9628facc Thanks!
            dongsheng Dongsheng Cai made changes -
            Link This issue will be resolved by PULL-377 [ PULL-377 ]
            Hide
            nebgor Aparup Banerjee added a comment -

            code looks good, saving works fine .

            need to fix the preview pages though as images aren't showing up there (ie: draft files)

            Show
            nebgor Aparup Banerjee added a comment - code looks good, saving works fine . need to fix the preview pages though as images aren't showing up there (ie: draft files)
            Hide
            nebgor Aparup Banerjee added a comment -

            Hi DS,
            the updated fix now works, draft files and saved files all showing up nice and well! it works for me!

            Show
            nebgor Aparup Banerjee added a comment - Hi DS, the updated fix now works, draft files and saved files all showing up nice and well! it works for me!
            Hide
            dongsheng Dongsheng Cai added a comment -

            PULL request submitted.

            Show
            dongsheng Dongsheng Cai added a comment - PULL request submitted.
            dongsheng Dongsheng Cai made changes -
            Status In Progress [ 3 ] Ready for review [ 10010 ]
            Resolution Fixed [ 1 ]
            Hide
            stronk7 Eloy Lafuente (stronk7) added a comment -

            Hi Dong, plz, take a look to my comment (when testing) at:

            http://tracker.moodle.org/browse/PULL-377?focusedCommentId=105851&page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-105851

            Feel free to reply there or here... TIA and ciao

            Show
            stronk7 Eloy Lafuente (stronk7) added a comment - Hi Dong, plz, take a look to my comment (when testing) at: http://tracker.moodle.org/browse/PULL-377?focusedCommentId=105851&page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-105851 Feel free to reply there or here... TIA and ciao
            dougiamas Martin Dougiamas made changes -
            Resolution Fixed [ 1 ]
            Status Ready for review [ 10010 ] Reopened [ 4 ]
            dougiamas Martin Dougiamas made changes -
            Fix Version/s STABLE Sprint 7 [ 10540 ]
            Fix Version/s STABLE Sprint 6 [ 10535 ]
            dougiamas Martin Dougiamas made changes -
            Link This issue is blocked by MDL-26739 [ MDL-26739 ]
            dougiamas Martin Dougiamas made changes -
            Fix Version/s STABLE Sprint 7 [ 10540 ]
            dougiamas Martin Dougiamas made changes -
            Fix Version/s STABLE backlog [ 10463 ]
            Hide
            tsala Helen Foster added a comment -

            Linking to MDL-26782 as both issues are about images in a wiki.

            Show
            tsala Helen Foster added a comment - Linking to MDL-26782 as both issues are about images in a wiki.
            tsala Helen Foster made changes -
            Link This issue has been marked as being related by MDL-26782 [ MDL-26782 ]
            dougiamas Martin Dougiamas made changes -
            Fix Version/s STABLE Sprint 8 [ 10541 ]
            Fix Version/s STABLE backlog [ 10463 ]
            dongsheng Dongsheng Cai made changes -
            Parent MDL-26739 [ 42833 ]
            Issue Type Bug [ 1 ] Sub-task [ 5 ]
            Hide
            granludo Ludo ( Marc Alier) added a comment -

            Hi,
            you guys did not like the way it was code the file uploading so you commented all the lines. If you provide some insight on how to do it we may add the feature again.
            L.

            Show
            granludo Ludo ( Marc Alier) added a comment - Hi, you guys did not like the way it was code the file uploading so you commented all the lines. If you provide some insight on how to do it we may add the feature again. L.
            dougiamas Martin Dougiamas made changes -
            Fix Version/s STABLE Sprint 9 [ 10550 ]
            Fix Version/s STABLE Sprint 8 [ 10541 ]
            Hide
            dongsheng Dongsheng Cai added a comment -

            Martin,

            Can you please review my changes?

            repo: git@github.com:dongsheng/moodle.git
            Branch: s8_MDL-26392_wiki_files
            diff: https://github.com/dongsheng/moodle/compare/master...s8_MDL-26392_wiki_files

            Show
            dongsheng Dongsheng Cai added a comment - Martin, Can you please review my changes? repo: git@github.com:dongsheng/moodle.git Branch: s8_ MDL-26392 _wiki_files diff: https://github.com/dongsheng/moodle/compare/master...s8_MDL-26392_wiki_files
            dongsheng Dongsheng Cai made changes -
            Status Reopened [ 4 ] In Progress [ 3 ]
            dongsheng Dongsheng Cai made changes -
            Link This issue will be resolved by PULL-613 [ PULL-613 ]
            dongsheng Dongsheng Cai made changes -
            Link This issue will be resolved by PULL-612 [ PULL-612 ]
            Hide
            dongsheng Dongsheng Cai added a comment -

            PULL requests submitted

            Show
            dongsheng Dongsheng Cai added a comment - PULL requests submitted
            dongsheng Dongsheng Cai made changes -
            Status In Progress [ 3 ] Ready for review [ 10010 ]
            Resolution Fixed [ 1 ]
            Hide
            skodak Petr Skoda added a comment -

            Reopening, see PULL for details.

            Show
            skodak Petr Skoda added a comment - Reopening, see PULL for details.
            skodak Petr Skoda made changes -
            Resolution Fixed [ 1 ]
            Status Ready for review [ 10010 ] Reopened [ 4 ]
            dougiamas Martin Dougiamas made changes -
            Fix Version/s STABLE Sprint 10 [ 10551 ]
            Fix Version/s STABLE Sprint 9 [ 10550 ]
            dougiamas Martin Dougiamas made changes -
            Workflow MDL Workflow [ 67733 ] MDL Full Workflow [ 76866 ]
            dongsheng Dongsheng Cai made changes -
            Status Reopened [ 4 ] Development in progress [ 3 ]
            Hide
            dongsheng Dongsheng Cai added a comment -

            1/ require_course_login() is not enough when modifying any text or file - luckily since 2.0 guests and not-logged-in do not have any write caps, but I think we should use the require_login() x require_course_login() correctly

            2/ you can not bump version above the later branch 2011040500, it has to use single +1 increments now that we branched

            3/ each file is using different "@package mod-wiki-2.0"

            4/ why can not students manage files by default?

            5/ please use s() when adding any html element attributes - such as in: $html .= "<img src=\"$CFG->wwwroot/mod/wiki/editors/wiki/images/$button[0]\" alt=\"".$button[1]."\" title=\"".$button[1]."\" />";

            6/ do not make empty line after initial "<?php" - it helps some IDEs a lot

            7/ $groupanduser = optional_param('groupanduser', 0, PARAM_TEXT); is definitely very bad coding style

            8/ mod/wiki/filesedit.php accepts both pageid and subwiki id, why? you can derive the subwiki from the pageid, but if you have both you need to verify they match - you must not use them separately without this validation; the same problem might be in other files

            9/ mod/wiki/filesedit.php why are all parameters optional? That does not make much sense to me.

            Show
            dongsheng Dongsheng Cai added a comment - 1/ require_course_login() is not enough when modifying any text or file - luckily since 2.0 guests and not-logged-in do not have any write caps, but I think we should use the require_login() x require_course_login() correctly 2/ you can not bump version above the later branch 2011040500, it has to use single +1 increments now that we branched 3/ each file is using different "@package mod-wiki-2.0" 4/ why can not students manage files by default? 5/ please use s() when adding any html element attributes - such as in: $html .= "<img src=\"$CFG->wwwroot/mod/wiki/editors/wiki/images/$button [0] \" alt=\"".$button [1] ."\" title=\"".$button [1] ."\" />"; 6/ do not make empty line after initial "<?php" - it helps some IDEs a lot 7/ $groupanduser = optional_param('groupanduser', 0, PARAM_TEXT); is definitely very bad coding style 8/ mod/wiki/filesedit.php accepts both pageid and subwiki id, why? you can derive the subwiki from the pageid, but if you have both you need to verify they match - you must not use them separately without this validation; the same problem might be in other files 9/ mod/wiki/filesedit.php why are all parameters optional? That does not make much sense to me.
            Hide
            dongsheng Dongsheng Cai added a comment -

            1. replaced all require_course_login with reqire_login
            2. fixed
            3. all files use '@package mod-wiki-2.0' now
            4. Martin agreed to not allowing student manage files by default
            5. fixed by using html_writer:empty_tag
            6. What IDEs are you talking about? And what's the benefits? I removed empty lines from the files I newly created, but most moodle files have that empty line.
            7. It's actually an existing problem of wiki 2, there is a subwiki selector created in render.php, it use groupid-userid as parameter for individual wiki in group mode, it affects other places as well, I am not sure if we should fix this render function in this issue, I added clean_param for now
            8. the page id only used for navigation, it's not for file management, if I don't pass it, users won't be able to go back to the page they came from.
            9. Fixed.

            Show
            dongsheng Dongsheng Cai added a comment - 1. replaced all require_course_login with reqire_login 2. fixed 3. all files use '@package mod-wiki-2.0' now 4. Martin agreed to not allowing student manage files by default 5. fixed by using html_writer:empty_tag 6. What IDEs are you talking about? And what's the benefits? I removed empty lines from the files I newly created, but most moodle files have that empty line. 7. It's actually an existing problem of wiki 2, there is a subwiki selector created in render.php, it use groupid-userid as parameter for individual wiki in group mode, it affects other places as well, I am not sure if we should fix this render function in this issue, I added clean_param for now 8. the page id only used for navigation, it's not for file management, if I don't pass it, users won't be able to go back to the page they came from. 9. Fixed.
            dongsheng Dongsheng Cai made changes -
            Status Development in progress [ 3 ] Waiting for peer review [ 10012 ]
            Pull Master Diff URL https://github.com/dongsheng/moodle/compare/master...s9_MDL-26392_wiki_files_master
            Pull Master Branch s9_MDL-26392_wiki_files_master
            Pull 2.0 Diff URL https://github.com/dongsheng/moodle/compare/MOODLE_20_STABLE...s9_MDL-26392_wiki_files_20
            Pull 2.0 Branch s9_MDL-26392_wiki_files_20
            Pull from Repository git://github.com/dongsheng/moodle.git
            Peer reviewer rwijaya
            Testing Instructions To test:
            1. Adding a new file using 'Files' tab in wiki
            2. Create a page using nwiki or creole format
            3. Select the file you uploaded in step 1
            4. save the page, you will be able to see the image in wik

            Please note:
            You need to test it using Collaborative wiki and Individual Wiki with group mode on/off.

            Thanks.
            Hide
            andyjdavis Andrew Davis added a comment -

            Is it worth retaining the commented out code in mod/wiki/filesedit_form.php starting at line 45?

            Show
            andyjdavis Andrew Davis added a comment - Is it worth retaining the commented out code in mod/wiki/filesedit_form.php starting at line 45?
            Hide
            dongsheng Dongsheng Cai added a comment -

            @Andrew

            Maybe not, I just removed it, two branches got updated.

            Show
            dongsheng Dongsheng Cai added a comment - @Andrew Maybe not, I just removed it, two branches got updated.
            Hide
            jerome Jérôme Mouneyrac added a comment -

            My peer review:
            It works. +1 for integration. I understood that some part of this code will be refactored later, so it's why I suggest to integrate it and to create issues for things to do later.

            I would suggest to write these issues:

            • some help information should explain clearly how to change the code [[image:Image|alt]]. Specially as clicking on the image button or selecting 'Insert an image...' would generate this code (a little bit buggy).
            • imo, for later, the button image should call the file picker and directly write the correct wiki code (as you would expect of anything related to file from a user point of view)
            Show
            jerome Jérôme Mouneyrac added a comment - My peer review: It works. +1 for integration. I understood that some part of this code will be refactored later, so it's why I suggest to integrate it and to create issues for things to do later. I would suggest to write these issues: some help information should explain clearly how to change the code [ [image:Image|alt] ]. Specially as clicking on the image button or selecting 'Insert an image...' would generate this code (a little bit buggy). imo, for later, the button image should call the file picker and directly write the correct wiki code (as you would expect of anything related to file from a user point of view)
            jerome Jérôme Mouneyrac made changes -
            Original Estimate 0 minutes [ 0 ]
            Remaining Estimate 0 minutes [ 0 ]
            Status Waiting for peer review [ 10012 ] Development in progress [ 3 ]
            jerome Jérôme Mouneyrac made changes -
            Status Development in progress [ 3 ] Waiting for integration review [ 10010 ]
            Hide
            stronk7 Eloy Lafuente (stronk7) 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
            stronk7 Eloy Lafuente (stronk7) 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
            dongsheng Dongsheng Cai added a comment -

            Rebased

            Regards,
            Dongsheng

            Show
            dongsheng Dongsheng Cai added a comment - Rebased Regards, Dongsheng
            stronk7 Eloy Lafuente (stronk7) made changes -
            Labels triaged ci pullweek-2011-19
            samhemelryk Sam Hemelryk made changes -
            Status Waiting for integration review [ 10010 ] Integration review in progress [ 10004 ]
            Integrator samhemelryk
            Hide
            samhemelryk Sam Hemelryk added a comment -

            Hmmmm I've just been reviewing this now and there are a couple of things I am not 100% sure about.

            1. The files link/tab is shown to everyone regardless of whether they can actually view the page.
            2. You are able to delete files that are still in use leading to broken images on pages where they were used.
            3. There is no way to access files from the HTML editor, I understand this isn't easy but it confused me while I was learning these changes.
            4. files.php - shouldn't pageid be a required param rather than optional given that the default 0 results in an exception anyway.
            5. filesedit.php displays all the tabs regardless of what the user can see/access (e.g. as a guest I am shown the edit tab).

            I also noted the following (although I wouldn't expect them to be fixed as part of this issue).

            1. The navigation is VERY confused for the wiki, especially how wiki_extend_navigation attempts to collect pageid and/or id from the URL... I think a better navigation solution could be found for the wiki module.
            2. The wiki still has inline JS calls (onchange) - these certainly need upgrading to make use of the page requirements API.
            3. There is a bug with the navigation when logged in as a guest and viewing a front page course module.
            4. The package phpdoc tag needs to be standardised and used consistently - there are still a couple of variations of the @package tag within the wiki module.

            Most of the points I have spotted are very minor or may be things that have already discussed with other reviewers, I'll wait to discuss this with Eloy before making a decision about integration.

            Cheers
            Sam

            Show
            samhemelryk Sam Hemelryk added a comment - Hmmmm I've just been reviewing this now and there are a couple of things I am not 100% sure about. The files link/tab is shown to everyone regardless of whether they can actually view the page. You are able to delete files that are still in use leading to broken images on pages where they were used. There is no way to access files from the HTML editor, I understand this isn't easy but it confused me while I was learning these changes. files.php - shouldn't pageid be a required param rather than optional given that the default 0 results in an exception anyway. filesedit.php displays all the tabs regardless of what the user can see/access (e.g. as a guest I am shown the edit tab). I also noted the following (although I wouldn't expect them to be fixed as part of this issue). The navigation is VERY confused for the wiki, especially how wiki_extend_navigation attempts to collect pageid and/or id from the URL... I think a better navigation solution could be found for the wiki module. The wiki still has inline JS calls (onchange) - these certainly need upgrading to make use of the page requirements API. There is a bug with the navigation when logged in as a guest and viewing a front page course module. The package phpdoc tag needs to be standardised and used consistently - there are still a couple of variations of the @package tag within the wiki module. Most of the points I have spotted are very minor or may be things that have already discussed with other reviewers, I'll wait to discuss this with Eloy before making a decision about integration. Cheers Sam
            Hide
            dougiamas Martin Dougiamas added a comment -

            A1, A4 and A5 should be fixed for sure. +1 to reject based on those. The others are probably other bugs.

            Show
            dougiamas Martin Dougiamas added a comment - A1, A4 and A5 should be fixed for sure. +1 to reject based on those. The others are probably other bugs.
            Hide
            samhemelryk Sam Hemelryk added a comment -

            Hi DS,

            I've just been talking this issue over with Martin and Eloy, its being rejected this week so that we can clean a couple of those things up and discuss the HTML editor and file deletion points if need be.

            Cheers
            Sam

            Show
            samhemelryk Sam Hemelryk added a comment - Hi DS, I've just been talking this issue over with Martin and Eloy, its being rejected this week so that we can clean a couple of those things up and discuss the HTML editor and file deletion points if need be. Cheers Sam
            samhemelryk Sam Hemelryk made changes -
            Status Integration review in progress [ 10004 ] Open [ 1 ]
            stronk7 Eloy Lafuente (stronk7) made changes -
            Status Open [ 1 ] Closed [ 6 ]
            Resolution Fixed [ 1 ]
            stronk7 Eloy Lafuente (stronk7) made changes -
            Resolution Fixed [ 1 ]
            Status Closed [ 6 ] Reopened [ 4 ]
            dougiamas Martin Dougiamas made changes -
            Fix Version/s Sprint to 2.1 [ 10650 ]
            Fix Version/s STABLE Sprint 10 [ 10551 ]
            stronk7 Eloy Lafuente (stronk7) made changes -
            Labels ci pullweek-2011-19 pullweek-2011-19
            Hide
            dongsheng Dongsheng Cai added a comment -

            Sam, Thanks for reviewing.

            A1. Fixed, user will need viewpage capability to view files
            A4. Fixed, pageid now being required
            A5. you are saying files.php right? filesedit.php doesn't display tabs, I added capability checks to tabs render.

            Regards,
            Dongsheng

            Show
            dongsheng Dongsheng Cai added a comment - Sam, Thanks for reviewing. A1. Fixed, user will need viewpage capability to view files A4. Fixed, pageid now being required A5. you are saying files.php right? filesedit.php doesn't display tabs, I added capability checks to tabs render. Regards, Dongsheng
            dongsheng Dongsheng Cai made changes -
            Status Reopened [ 4 ] Development in progress [ 3 ]
            dongsheng Dongsheng Cai made changes -
            Status Development in progress [ 3 ] Waiting for peer review [ 10012 ]
            Hide
            rwijaya Rossiani Wijaya added a comment -

            Hi Dongsheng,

            I tested both patches and received the following messages:

            Notice: Undefined variable: newcontent in /m20/moodle/mod/wiki/edit.php on line 88

            Notice: Undefined variable: context in /m20/moodle/mod/wiki/pagelib.php on line 1916

            Notice: Trying to get property of non-object in /m20/moodle/mod/wiki/pagelib.php on line 1916

            To reproduce:
            1. create new wiki (either creole or nwiki)
            2. upload img file
            3. add uploaded image on edit page and save.

            also notice mod/wiki/edit.php on line 48:
            [code]
            $newconent = '';
            if (!empty($newcontent) && is_array($newcontent)) {
            $newcontent = $newcontent['text'];
            }
            [/code]
            Variable $newconent, is this a typo?
            if it is, then what is the use of if statement for?

            Show
            rwijaya Rossiani Wijaya added a comment - Hi Dongsheng, I tested both patches and received the following messages: Notice: Undefined variable: newcontent in /m20/moodle/mod/wiki/edit.php on line 88 Notice: Undefined variable: context in /m20/moodle/mod/wiki/pagelib.php on line 1916 Notice: Trying to get property of non-object in /m20/moodle/mod/wiki/pagelib.php on line 1916 To reproduce: 1. create new wiki (either creole or nwiki) 2. upload img file 3. add uploaded image on edit page and save. also notice mod/wiki/edit.php on line 48: [code] $newconent = ''; if (!empty($newcontent) && is_array($newcontent)) { $newcontent = $newcontent ['text'] ; } [/code] Variable $newconent, is this a typo? if it is, then what is the use of if statement for?
            Hide
            dongsheng Dongsheng Cai added a comment -

            Rossiani
            Those errors are not from this patch, can you please file a new bug for it?

            Show
            dongsheng Dongsheng Cai added a comment - Rossiani Those errors are not from this patch, can you please file a new bug for it?
            Hide
            rwijaya Rossiani Wijaya added a comment -

            File a new issue for the errors I described above: MDL-27525

            Show
            rwijaya Rossiani Wijaya added a comment - File a new issue for the errors I described above: MDL-27525
            Hide
            rwijaya Rossiani Wijaya added a comment -

            Hi DS,

            Apart from the errors I mentioned above, adding images in nwiki and creole works great.

            Note: to test this issue, I set 'debug messages' to none.

            Show
            rwijaya Rossiani Wijaya added a comment - Hi DS, Apart from the errors I mentioned above, adding images in nwiki and creole works great. Note: to test this issue, I set 'debug messages' to none.
            rwijaya Rossiani Wijaya made changes -
            Status Waiting for peer review [ 10012 ] Development in progress [ 3 ]
            Hide
            dongsheng Dongsheng Cai added a comment -

            Thanks Rossiani

            I just assigned MDL-27525 myself.

            Show
            dongsheng Dongsheng Cai added a comment - Thanks Rossiani I just assigned MDL-27525 myself.
            stronk7 Eloy Lafuente (stronk7) made changes -
            Integration date 11/May/11
            Hide
            dongsheng Dongsheng Cai added a comment -

            Rossiani did you clicked "looks great to me" after peer review, this issue rolled back to development in progress

            Show
            dongsheng Dongsheng Cai added a comment - Rossiani did you clicked "looks great to me" after peer review, this issue rolled back to development in progress
            dongsheng Dongsheng Cai made changes -
            Status Development in progress [ 3 ] Waiting for peer review [ 10012 ]
            Hide
            rwijaya Rossiani Wijaya added a comment -

            Ds,

            I did click on "looks great to me" button after I reviewed it.

            Show
            rwijaya Rossiani Wijaya added a comment - Ds, I did click on "looks great to me" button after I reviewed it.
            Hide
            dongsheng Dongsheng Cai added a comment -

            Hi Rossiani

            Aparup just explained to me, either button will send issue back to in progress status, we need to click "send for integration".

            Show
            dongsheng Dongsheng Cai added a comment - Hi Rossiani Aparup just explained to me, either button will send issue back to in progress status, we need to click "send for integration".
            stronk7 Eloy Lafuente (stronk7) made changes -
            Integration date 11/May/11
            Hide
            rwijaya Rossiani Wijaya added a comment -

            Hi DS,

            This is look great. I'm sending this to integration.

            Show
            rwijaya Rossiani Wijaya added a comment - Hi DS, This is look great. I'm sending this to integration.
            rwijaya Rossiani Wijaya made changes -
            Status Waiting for peer review [ 10012 ] Waiting for integration review [ 10010 ]
            Hide
            dongsheng Dongsheng Cai added a comment -

            rebased

            Show
            dongsheng Dongsheng Cai added a comment - rebased
            samhemelryk Sam Hemelryk made changes -
            Currently in integration Yes
            Hide
            samhemelryk Sam Hemelryk added a comment -

            Hi Dongsheng,

            I've been reviewing this now for integration.
            All in all it looks good, I did spot several minor things but I think these could be cleaned up during integration and I've attached a patch to see them cleaned up.

            Once I've checked with Eloy that it is OK for things to be sent upstream this can go in.

            Cheers
            Sam

            Show
            samhemelryk Sam Hemelryk added a comment - Hi Dongsheng, I've been reviewing this now for integration. All in all it looks good, I did spot several minor things but I think these could be cleaned up during integration and I've attached a patch to see them cleaned up. Once I've checked with Eloy that it is OK for things to be sent upstream this can go in. Cheers Sam
            samhemelryk Sam Hemelryk made changes -
            Attachment MDL.26739.integration.patch [ 24023 ]
            Hide
            samhemelryk Sam Hemelryk added a comment -

            Forgot to mention what I spotted!

            Minor things to be corrected:

            • Two new strings insertimage and insertimage_help need to be added in alphabetically.
            • wiki_print_subwiki_selector $pagetype should default to 'view' and $baseurl is assumed a moodle_url and should never default to null.
            • wiki_files_tree should be declared public as per coding style
            • wiki_parser_real_path arguments have changes but PHPdoc hasn't been updated.

            Questions I have for you DS that I needed answered before this can be integrated:

            • Is the new string cannotmanagefiles used any where/needed?
            • Is the new string insertimage_help used any where/needed?

            And general notes:

            • Test on frontpage wiki module and check access when not logged in.
            • HTML generation through $OUTPUT should be moved out of loops where possible to aid page generation time, see mod_wiki_renderer::htmllize_tree (the pix_icon calls).
            Show
            samhemelryk Sam Hemelryk added a comment - Forgot to mention what I spotted! Minor things to be corrected: Two new strings insertimage and insertimage_help need to be added in alphabetically. wiki_print_subwiki_selector $pagetype should default to 'view' and $baseurl is assumed a moodle_url and should never default to null. wiki_files_tree should be declared public as per coding style wiki_parser_real_path arguments have changes but PHPdoc hasn't been updated. Questions I have for you DS that I needed answered before this can be integrated: Is the new string cannotmanagefiles used any where/needed? Is the new string insertimage_help used any where/needed? And general notes: Test on frontpage wiki module and check access when not logged in. HTML generation through $OUTPUT should be moved out of loops where possible to aid page generation time, see mod_wiki_renderer::htmllize_tree (the pix_icon calls).
            samhemelryk Sam Hemelryk made changes -
            Status Waiting for integration review [ 10010 ] Integration review in progress [ 10004 ]
            Hide
            samhemelryk Sam Hemelryk added a comment -

            Thanks Dongsheng this has been integrated now... Hoorah

            Show
            samhemelryk Sam Hemelryk added a comment - Thanks Dongsheng this has been integrated now... Hoorah
            samhemelryk Sam Hemelryk made changes -
            Status Integration review in progress [ 10004 ] Waiting for testing [ 10005 ]
            stronk7 Eloy Lafuente (stronk7) made changes -
            Status Waiting for testing [ 10005 ] Testing in progress [ 10011 ]
            Tester stronk7
            Hide
            stronk7 Eloy Lafuente (stronk7) added a comment -

            passing based one Rossiani's comments.

            Show
            stronk7 Eloy Lafuente (stronk7) added a comment - passing based one Rossiani's comments.
            stronk7 Eloy Lafuente (stronk7) made changes -
            Status Testing in progress [ 10011 ] Tested [ 10006 ]
            Hide
            stronk7 Eloy Lafuente (stronk7) added a comment -

            This is now upstream, yay! Many thanks!

            Show
            stronk7 Eloy Lafuente (stronk7) added a comment - This is now upstream, yay! Many thanks!
            stronk7 Eloy Lafuente (stronk7) made changes -
            Status Tested [ 10006 ] Closed [ 6 ]
            Fix Version/s 2.0.4 [ 10652 ]
            Resolution Fixed [ 1 ]
            Currently in integration Yes
            Integration date 9/Jun/11
            stronk7 Eloy Lafuente (stronk7) made changes -
            Fix Version/s DEV Sprint 2.1 [ 10650 ]
            Subversion JIRA

            Links Hierarchy

             Documentation

            Invalid license: EXPIRED

              People

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

                Dates

                • Created:
                  Updated:
                  Resolved:
                  Fix Release Date:
                  1/Aug/11