Uploaded image for project: 'Moodle'
  1. Moodle
  2. MDL-42023

Edit PDF: Integrate editpdf plugin for mod_assign

    Details

    • Type: Task
    • Status: Closed
    • Priority: Minor
    • Resolution: Fixed
    • Affects Version/s: 2.6
    • Fix Version/s: 2.6, FRONTEND
    • Component/s: Assignment
    • Labels:
    • Testing Instructions:
      Hide
      1. Go to the admin settings page for the annotate pdf plugin.
      2. Make sure ghostscript is installed on your system and the path is correct in the setting.
      3. Test the ghostscript path and make sure the test image displays.
      4. Upload some stamps
      5. Create an assignment with submission files enabled, and annotate pdf (feedback) enabled.
      6. Login as a student and add a pdf file as your submission to the assignment
      7. Login as a teacher and grade the student (not quickgrade)
      8. The work flow is to click "Launch PDF Editor" on the grading page to start the editor - changes are automatically saved as you go - but are not sent to the student until you click save on the grading page which also generates the feedback pdf.
      9. Do some exploratory testing in different browsers.
      10. Backup the assignment
      11. Delete some stamps that were used in the assignment
      12. Restore the assignment
      13. Verify the stamps still display in the assignment

      Known issues at this time:
      Behat tests. We will work on behat tests for this - but it will probably require changes to behat for 2 reasons - the first is that the tests should be skipped if ghostscript is not installed/available. Currently skipped tests in behat count pretty much like failures (according to David) so we will need some better behaviour here if we are to add tests. The second reason is that in order to draw etc, you need click and drag - which doesn't work very well with behat. (There are unit tests).

      RTL support. We still have an open issue to add extra fonts and some RTL switching behaviour for comments. We will get this finished for the release.

      Background processing. Would be lovely - but there are big cron/locking issues to finish first (see the chain from MDL-25500). I have done heaps of work on these but they will not be ready for 2.6.

      Icon colours. There is an open issue to finalise the colours of the icons for comments.

      Show
      Go to the admin settings page for the annotate pdf plugin. Make sure ghostscript is installed on your system and the path is correct in the setting. Test the ghostscript path and make sure the test image displays. Upload some stamps Create an assignment with submission files enabled, and annotate pdf (feedback) enabled. Login as a student and add a pdf file as your submission to the assignment Login as a teacher and grade the student (not quickgrade) The work flow is to click "Launch PDF Editor" on the grading page to start the editor - changes are automatically saved as you go - but are not sent to the student until you click save on the grading page which also generates the feedback pdf. Do some exploratory testing in different browsers. Backup the assignment Delete some stamps that were used in the assignment Restore the assignment Verify the stamps still display in the assignment Known issues at this time: Behat tests. We will work on behat tests for this - but it will probably require changes to behat for 2 reasons - the first is that the tests should be skipped if ghostscript is not installed/available. Currently skipped tests in behat count pretty much like failures (according to David) so we will need some better behaviour here if we are to add tests. The second reason is that in order to draw etc, you need click and drag - which doesn't work very well with behat. (There are unit tests). RTL support. We still have an open issue to add extra fonts and some RTL switching behaviour for comments. We will get this finished for the release. Background processing. Would be lovely - but there are big cron/locking issues to finish first (see the chain from MDL-25500 ). I have done heaps of work on these but they will not be ready for 2.6. Icon colours. There is an open issue to finalise the colours of the icons for comments.
    • Affected Branches:
      MOODLE_26_STABLE
    • Fixed Branches:
      MOODLE_26_STABLE
    • Pull from Repository:
    • Pull Master Branch:
      MDL-42023-master
    • Sprint:
      FRONTEND Sprint 5
    • Story Points (Obsolete):
      13
    • Sprint:
      FRONTEND Sprint 5

      Description

      This task is to integrate the initial version of the editpdf plugin. There are a few issues in the Epic that will be done after the release of 2.6 (particularly the background processing) so we want to keep the epic open and make this first version one of the sub tasks.

        Gliffy Diagrams

        1. brokenpdfsshot.png
          190 kB
        2. firefox_top.png
          11 kB
        3. testgs.png
          91 kB

          Issue Links

            Activity

            Hide
            jerome Jérôme Mouneyrac added a comment - - edited

            I didn't find any major issues. Sending it to integration to have a integration review before code freeze.
            Known issues are listed there: https://tracker.moodle.org/browse/MDL-38171

            Show
            jerome Jérôme Mouneyrac added a comment - - edited I didn't find any major issues. Sending it to integration to have a integration review before code freeze. Known issues are listed there: https://tracker.moodle.org/browse/MDL-38171
            Hide
            marina Marina Glancy added a comment -

            Attaching the screenshot of "toolbar" in firefox (until I tried in Chrome I did not even understand it was a toolbar)

            Show
            marina Marina Glancy added a comment - Attaching the screenshot of "toolbar" in firefox (until I tried in Chrome I did not even understand it was a toolbar)
            Hide
            marina Marina Glancy added a comment -

            Wow, when I finally made it working - it looks awesome.

            But my comments:

            Code review:

            Overall code looks great, awesome usage of autoloader too.

            1. Changes in admin_setting_configstoredfile look strange. Can't find any code using this 'jsonfilenames'
            2. I can see that backup is implemented. Great! Can you please add testing instructions for backup/restore including situation when annotation used stamps but they are not present during restoration.
            3. Not really sure why classes assignfeedback_editpdf\annotation and comment need to extend stdClass and why functions assignfeedback_editpdf\page_editor::annotation_from_record() and comment_from_record() are not part of the original classes (or even better constructors).
            4. I can see various errors in phpdocs mod/assign/feedback/editpdf/classes/document_services.php: namespace \ is not indicated for \stored_file, \assign, etc. @return array(stored_file) should be @return \stored_file[], params don't match the function parameters, etc.
            5. assignfeedback_editpdf_renderer::get_shortcut() - this is completely random set of letters. Btw Alt+Shift works, Alt+Ctrl - does not. Not sure what is "Option"
            6. assignfeedback_editpdf_pluginfile(): I have not found any usage of 'revision' for stamps. Also some debugging left in code: error_log(print_r($fullpath, true));
            7. Sorry, did not look at JS files yet

            Testing (you might be already aware of those):

            1. Could not make working in Firefox - see above - this must be fixed before integration
            2. Errors on GS testing page - see screenshot - this must be fixed before integration
            3. Error in PDF file resulted in non-stop loading icon - this must be fixed before integration
            4. Landscape orientation or size bigger than A4 is not recognised, text outside the window unreachable, no scrolling/scaling present.
            5. Window is neither resizable nor movable, it appears low on the screen and I have to scroll the page to see it. But after that opening/closing any subwindow the scrolling jumps on top of the page
            6. Right mouse button click works as left mouse click (plus displays browser context menu), right mouse button release is not caught resulting in mess.
            7. Current page number is not displayed
            8. Highlighting is not intuitive (I would expect the left middle point to be on mouse level, not top left corner)
            9. I was able to add comment to the quick list but can't insert one into into empty comment - if I click the context icon and then select quick comment the empty comment box disappears
            10. I predict "Undo" to be the most desired new feature
            11. Can I see the annotated feedback as a student? Could not find how

            Attempt to save an annotated PDF with stamps resulted multiple errors, the unique ones are:

            ( ! ) Warning: Division by zero in /home/marina/repositories/int_master/integration/lib/tcpdf/tcpdf.php on line 11690
            Call Stack
            #	Time	Memory	Function	Location
            1	0.0017	250760	{main}( )	../view.php:0
            2	0.3700	36340360	assign->view( )	../view.php:53
            3	0.3701	36342008	assign->process_save_grade( )	../locallib.php:450
            4	0.4808	39366928	assign->save_grade( )	../locallib.php:6326
            5	0.4814	39369352	assign->apply_grade_to_user( )	../locallib.php:6225
            6	0.4865	39399800	assign_feedback_editpdf->save( )	../locallib.php:6119
            7	0.4865	39399880	assignfeedback_editpdf\document_services::generate_feedback_document( )	../locallib.php:149
            8	0.8869	54043456	assignfeedback_editpdf\pdf->add_annotation( )	../document_services.php:435
            9	0.8870	54044408	TCPDF->Ellipse( )	../pdf.php:283
            10	0.8870	54045328	TCPDF->_outellipticalarc( )	../tcpdf.php:11646
             
            ( ! ) Warning: Division by zero in /home/marina/repositories/int_master/integration/lib/tcpdf/tcpdf.php on line 11691
            Call Stack
            #	Time	Memory	Function	Location
            1	0.0017	250760	{main}( )	../view.php:0
            2	0.3700	36340360	assign->view( )	../view.php:53
            3	0.3701	36342008	assign->process_save_grade( )	../locallib.php:450
            4	0.4808	39366928	assign->save_grade( )	../locallib.php:6326
            5	0.4814	39369352	assign->apply_grade_to_user( )	../locallib.php:6225
            6	0.4865	39399800	assign_feedback_editpdf->save( )	../locallib.php:6119
            7	0.4865	39399880	assignfeedback_editpdf\document_services::generate_feedback_document( )	../locallib.php:149
            8	0.8869	54043456	assignfeedback_editpdf\pdf->add_annotation( )	../document_services.php:435
            9	0.8870	54044408	TCPDF->Ellipse( )	../pdf.php:283
            10	0.8870	54045328	TCPDF->_outellipticalarc( )	../tcpdf.php:11646
             
            ( ! ) Warning: opendir(/home/marina/repositories/int_master/integration/mod/assign/feedback/pdf/pix/stamps): failed to open dir: No such file or directory in /home/marina/repositories/int_master/integration/mod/assign/feedback/editpdf/classes/pdf.php on line 342
            Call Stack
            #	Time	Memory	Function	Location
            1	0.0017	250760	{main}( )	../view.php:0
            2	0.3700	36340360	assign->view( )	../view.php:53
            3	0.3701	36342008	assign->process_save_grade( )	../locallib.php:450
            4	0.4808	39366928	assign->save_grade( )	../locallib.php:6326
            5	0.4814	39369352	assign->apply_grade_to_user( )	../locallib.php:6225
            6	0.4865	39399800	assign_feedback_editpdf->save( )	../locallib.php:6119
            7	0.4865	39399880	assignfeedback_editpdf\document_services::generate_feedback_document( )	../locallib.php:149
            8	0.8946	54053440	assignfeedback_editpdf\pdf->add_annotation( )	../document_services.php:435
            9	0.8946	54054376	assignfeedback_editpdf\pdf::get_stamp_file( )	../pdf.php:313
            10	0.8946	54054376	assignfeedback_editpdf\pdf::get_stamps( )	../pdf.php:363
            11	0.8946	54054576	opendir ( )	../pdf.php:342

            Show
            marina Marina Glancy added a comment - Wow, when I finally made it working - it looks awesome. But my comments: Code review: Overall code looks great, awesome usage of autoloader too. Changes in admin_setting_configstoredfile look strange. Can't find any code using this 'jsonfilenames' I can see that backup is implemented. Great! Can you please add testing instructions for backup/restore including situation when annotation used stamps but they are not present during restoration. Not really sure why classes assignfeedback_editpdf\annotation and comment need to extend stdClass and why functions assignfeedback_editpdf\page_editor::annotation_from_record() and comment_from_record() are not part of the original classes (or even better constructors). I can see various errors in phpdocs mod/assign/feedback/editpdf/classes/document_services.php: namespace \ is not indicated for \stored_file, \assign, etc. @return array(stored_file) should be @return \stored_file[], params don't match the function parameters, etc. assignfeedback_editpdf_renderer::get_shortcut() - this is completely random set of letters. Btw Alt+Shift works, Alt+Ctrl - does not. Not sure what is "Option" assignfeedback_editpdf_pluginfile(): I have not found any usage of 'revision' for stamps. Also some debugging left in code: error_log(print_r($fullpath, true)); Sorry, did not look at JS files yet Testing (you might be already aware of those): Could not make working in Firefox - see above - this must be fixed before integration Errors on GS testing page - see screenshot - this must be fixed before integration Error in PDF file resulted in non-stop loading icon - this must be fixed before integration Landscape orientation or size bigger than A4 is not recognised, text outside the window unreachable, no scrolling/scaling present. Window is neither resizable nor movable, it appears low on the screen and I have to scroll the page to see it. But after that opening/closing any subwindow the scrolling jumps on top of the page Right mouse button click works as left mouse click (plus displays browser context menu), right mouse button release is not caught resulting in mess. Current page number is not displayed Highlighting is not intuitive (I would expect the left middle point to be on mouse level, not top left corner) I was able to add comment to the quick list but can't insert one into into empty comment - if I click the context icon and then select quick comment the empty comment box disappears I predict "Undo" to be the most desired new feature Can I see the annotated feedback as a student? Could not find how Attempt to save an annotated PDF with stamps resulted multiple errors, the unique ones are: ( ! ) Warning: Division by zero in /home/marina/repositories/int_master/integration/lib/tcpdf/tcpdf.php on line 11690 Call Stack # Time Memory Function Location 1 0.0017 250760 {main}( ) ../view.php:0 2 0.3700 36340360 assign->view( ) ../view.php:53 3 0.3701 36342008 assign->process_save_grade( ) ../locallib.php:450 4 0.4808 39366928 assign->save_grade( ) ../locallib.php:6326 5 0.4814 39369352 assign->apply_grade_to_user( ) ../locallib.php:6225 6 0.4865 39399800 assign_feedback_editpdf->save( ) ../locallib.php:6119 7 0.4865 39399880 assignfeedback_editpdf\document_services::generate_feedback_document( ) ../locallib.php:149 8 0.8869 54043456 assignfeedback_editpdf\pdf->add_annotation( ) ../document_services.php:435 9 0.8870 54044408 TCPDF->Ellipse( ) ../pdf.php:283 10 0.8870 54045328 TCPDF->_outellipticalarc( ) ../tcpdf.php:11646   ( ! ) Warning: Division by zero in /home/marina/repositories/int_master/integration/lib/tcpdf/tcpdf.php on line 11691 Call Stack # Time Memory Function Location 1 0.0017 250760 {main}( ) ../view.php:0 2 0.3700 36340360 assign->view( ) ../view.php:53 3 0.3701 36342008 assign->process_save_grade( ) ../locallib.php:450 4 0.4808 39366928 assign->save_grade( ) ../locallib.php:6326 5 0.4814 39369352 assign->apply_grade_to_user( ) ../locallib.php:6225 6 0.4865 39399800 assign_feedback_editpdf->save( ) ../locallib.php:6119 7 0.4865 39399880 assignfeedback_editpdf\document_services::generate_feedback_document( ) ../locallib.php:149 8 0.8869 54043456 assignfeedback_editpdf\pdf->add_annotation( ) ../document_services.php:435 9 0.8870 54044408 TCPDF->Ellipse( ) ../pdf.php:283 10 0.8870 54045328 TCPDF->_outellipticalarc( ) ../tcpdf.php:11646   ( ! ) Warning: opendir(/home/marina/repositories/int_master/integration/mod/assign/feedback/pdf/pix/stamps): failed to open dir: No such file or directory in /home/marina/repositories/int_master/integration/mod/assign/feedback/editpdf/classes/pdf.php on line 342 Call Stack # Time Memory Function Location 1 0.0017 250760 {main}( ) ../view.php:0 2 0.3700 36340360 assign->view( ) ../view.php:53 3 0.3701 36342008 assign->process_save_grade( ) ../locallib.php:450 4 0.4808 39366928 assign->save_grade( ) ../locallib.php:6326 5 0.4814 39369352 assign->apply_grade_to_user( ) ../locallib.php:6225 6 0.4865 39399800 assign_feedback_editpdf->save( ) ../locallib.php:6119 7 0.4865 39399880 assignfeedback_editpdf\document_services::generate_feedback_document( ) ../locallib.php:149 8 0.8946 54053440 assignfeedback_editpdf\pdf->add_annotation( ) ../document_services.php:435 9 0.8946 54054376 assignfeedback_editpdf\pdf::get_stamp_file( ) ../pdf.php:313 10 0.8946 54054376 assignfeedback_editpdf\pdf::get_stamps( ) ../pdf.php:363 11 0.8946 54054576 opendir ( ) ../pdf.php:342
            Hide
            damyon Damyon Wiese added a comment -

            Thanks Marina,

            I'll get started on the fixes right now.

            Some quick comments from your review:

            code_review 1. Ah - I didn't realise Jerome added that - it's not used - I'll axe it.

            code_review 5. Jerome picked the list of shortcuts by looking at the list of keys already used by the various screenreaders and picking from what was left in order to avoid accessibility issues. They kind of map to the visual order of the buttons rather than anything to do with the button names.

            I'll add fixes ontop of the branch - but would prefer to squash once it has the all clear.

            Show
            damyon Damyon Wiese added a comment - Thanks Marina, I'll get started on the fixes right now. Some quick comments from your review: code_review 1. Ah - I didn't realise Jerome added that - it's not used - I'll axe it. code_review 5. Jerome picked the list of shortcuts by looking at the list of keys already used by the various screenreaders and picking from what was left in order to avoid accessibility issues. They kind of map to the visual order of the buttons rather than anything to do with the button names. I'll add fixes ontop of the branch - but would prefer to squash once it has the all clear.
            Hide
            damyon Damyon Wiese added a comment -

            Thanks Marina,

            I've pushed a set of fixes now.

            A summary of the changes from your points above:
            code review:
            1. Removed
            2. Testing instructions added - and I changed the code so the stamps are copied to the feedback and kept with backup/restore. So restore to a different site will work.
            3. Changed.
            4. All phpdocs errors (except testing and thrid party code) are fixed
            5. Not changed - but see comment above for reasoning.
            6. pluginfile handling is simplified (because the stamps belong to the feedback now)
            7. OK - (note: they are the bulk of this code)

            Testing:
            1. I think this was a window size issue - maybe triggered by the lack of stamps. Anyway - I've changed the toolbar to wrap if required so this shouldn't happen.
            2. Fixed
            3. Fixed
            4. Not changed - this can be a separate issue
            5. Not changed - this can be a separate issue
            6. Fixed
            7. There is already a separate issue for that
            8. Not changed - this can be a separate issue
            9. Not changed - this can be a separate issue
            10. Not changed - this can be a separate issue
            11. Students can either download the pdf or view the annotations online from the front page of the assignment (But only once the feedback has been "saved" by clicking save on the grading form).

            Show
            damyon Damyon Wiese added a comment - Thanks Marina, I've pushed a set of fixes now. A summary of the changes from your points above: code review: 1. Removed 2. Testing instructions added - and I changed the code so the stamps are copied to the feedback and kept with backup/restore. So restore to a different site will work. 3. Changed. 4. All phpdocs errors (except testing and thrid party code) are fixed 5. Not changed - but see comment above for reasoning. 6. pluginfile handling is simplified (because the stamps belong to the feedback now) 7. OK - (note: they are the bulk of this code) Testing: 1. I think this was a window size issue - maybe triggered by the lack of stamps. Anyway - I've changed the toolbar to wrap if required so this shouldn't happen. 2. Fixed 3. Fixed 4. Not changed - this can be a separate issue 5. Not changed - this can be a separate issue 6. Fixed 7. There is already a separate issue for that 8. Not changed - this can be a separate issue 9. Not changed - this can be a separate issue 10. Not changed - this can be a separate issue 11. Students can either download the pdf or view the annotations online from the front page of the assignment (But only once the feedback has been "saved" by clicking save on the grading form).
            Hide
            marina Marina Glancy added a comment -

            Thanks Damyon for quick fixes. I asked Andrew Nicols to look at JS as well. I re-start my reviewing and testing myself now.

            Show
            marina Marina Glancy added a comment - Thanks Damyon for quick fixes. I asked Andrew Nicols to look at JS as well. I re-start my reviewing and testing myself now.
            Hide
            marina Marina Glancy added a comment -

            Hi Damyon,
            thanks, the most issues seem to be fixed but some still remain.

            1. Broken pdf file results in 500 error in AJAX call with following JS error:
            http://localhost/int_master/mod/assign/feedback/editpdf/ajax.php?sesskey=VM3qDNehpI&action=loadallpages&userid=6&attemptnumber=0&assignmentid=6 – 500 Internal Server Error
            TypeError: constructor.NAME is undefined
            ...._cssPrefix = constructor.CSS_PREFIX || _getClassName(constructor.NAME.toLowerCa...

            2. After saving the changes I get an error in console:
            link.on('click', this.link_handler, this);
            (this is a page saying "The grade changes were saved" and there is no "annotate pdf" link there)

            3. On grading page the link "Launch PDF editor..." appears only for the files that I have already graded using "annotate pdf". New pdf submissions don't have this link. If I click this link from grading page, I don't get to see toolbar.
            This is a new one, I did not notice it last time, I don't mind if you create a separate issue for it

            4. I tried to update the grade on submission with broken pdf and received an error:

            ( ! ) Fatal error: Maximum function nesting level of '100' reached, aborting! in /home/marina/repositories/int_master/integration/mod/assign/feedback/editpdf/fpdi/pdf_parser.php on line 617
            Call Stack
            #	Time	Memory	Function	Location
            1	0.0008	250816	{main}( )	../view.php:0
            2	0.3317	36400704	assign->view( )	../view.php:53
            3	0.3317	36402352	assign->process_save_grade( )	../locallib.php:450
            4	0.4273	39486624	assign->save_grade( )	../locallib.php:6326
            5	0.4279	39489048	assign->apply_grade_to_user( )	../locallib.php:6225
            6	0.4352	39508368	assign_feedback_editpdf->save( )	../locallib.php:6119
            7	0.4352	39508448	assignfeedback_editpdf\document_services::generate_feedback_document( )	../locallib.php:184
            8	0.5341	54319504	assignfeedback_editpdf\pdf->set_pdf( )	../document_services.php:435
            9	0.5342	54319616	assignfeedback_editpdf\pdf->load_pdf( )	../pdf.php:145
            10	0.5346	54323456	FPDI->setSourceFile( )	../pdf.php:130
            11	0.5346	54323600	FPDI->_getPdfParser( )	../fpdi.php:90
            12	0.5346	54323896	fpdi_pdf_parser->fpdi_pdf_parser( )	../fpdi.php:103
            13	0.5375	54361392	fpdi_pdf_parser->read_pages( )	../fpdi_pdf_parser.php:78
            14	0.5381	54365736	fpdi_pdf_parser->read_pages( )	../fpdi_pdf_parser.php:403
            15	0.5386	54369984	fpdi_pdf_parser->read_pages( )	../fpdi_pdf_parser.php:403
            16	0.5391	54374232	fpdi_pdf_parser->read_pages( )	../fpdi_pdf_parser.php:403
            ....
            94	0.5625	54836560	fpdi_pdf_parser->read_pages( )	../fpdi_pdf_parser.php:403
            95	0.5625	54836864	pdf_parser->pdf_resolve_object( )	../fpdi_pdf_parser.php:399
            96	0.5626	54837544	pdf_parser->pdf_read_value( )	../pdf_parser.php:577
            97	0.5626	54838536	pdf_parser->pdf_read_value( )	../pdf_parser.php:381
            98	0.5626	54838752	pdf_parser->pdf_read_value( )	../pdf_parser.php:408
            99	0.5627	54838752	pdf_parser->pdf_read_token( )	../pdf_parser.php:476

            5. Still unable to view the annotation as student.

            6. Too much debugging information in console window with dumping objects

            Show
            marina Marina Glancy added a comment - Hi Damyon, thanks, the most issues seem to be fixed but some still remain. 1. Broken pdf file results in 500 error in AJAX call with following JS error: http://localhost/int_master/mod/assign/feedback/editpdf/ajax.php?sesskey=VM3qDNehpI&action=loadallpages&userid=6&attemptnumber=0&assignmentid=6 – 500 Internal Server Error TypeError: constructor.NAME is undefined ...._cssPrefix = constructor.CSS_PREFIX || _getClassName(constructor.NAME.toLowerCa... 2. After saving the changes I get an error in console: link.on('click', this.link_handler, this); (this is a page saying "The grade changes were saved" and there is no "annotate pdf" link there) 3. On grading page the link "Launch PDF editor..." appears only for the files that I have already graded using "annotate pdf". New pdf submissions don't have this link. If I click this link from grading page, I don't get to see toolbar. This is a new one, I did not notice it last time, I don't mind if you create a separate issue for it 4. I tried to update the grade on submission with broken pdf and received an error: ( ! ) Fatal error: Maximum function nesting level of '100' reached, aborting! in /home/marina/repositories/int_master/integration/mod/assign/feedback/editpdf/fpdi/pdf_parser.php on line 617 Call Stack # Time Memory Function Location 1 0.0008 250816 {main}( ) ../view.php:0 2 0.3317 36400704 assign->view( ) ../view.php:53 3 0.3317 36402352 assign->process_save_grade( ) ../locallib.php:450 4 0.4273 39486624 assign->save_grade( ) ../locallib.php:6326 5 0.4279 39489048 assign->apply_grade_to_user( ) ../locallib.php:6225 6 0.4352 39508368 assign_feedback_editpdf->save( ) ../locallib.php:6119 7 0.4352 39508448 assignfeedback_editpdf\document_services::generate_feedback_document( ) ../locallib.php:184 8 0.5341 54319504 assignfeedback_editpdf\pdf->set_pdf( ) ../document_services.php:435 9 0.5342 54319616 assignfeedback_editpdf\pdf->load_pdf( ) ../pdf.php:145 10 0.5346 54323456 FPDI->setSourceFile( ) ../pdf.php:130 11 0.5346 54323600 FPDI->_getPdfParser( ) ../fpdi.php:90 12 0.5346 54323896 fpdi_pdf_parser->fpdi_pdf_parser( ) ../fpdi.php:103 13 0.5375 54361392 fpdi_pdf_parser->read_pages( ) ../fpdi_pdf_parser.php:78 14 0.5381 54365736 fpdi_pdf_parser->read_pages( ) ../fpdi_pdf_parser.php:403 15 0.5386 54369984 fpdi_pdf_parser->read_pages( ) ../fpdi_pdf_parser.php:403 16 0.5391 54374232 fpdi_pdf_parser->read_pages( ) ../fpdi_pdf_parser.php:403 .... 94 0.5625 54836560 fpdi_pdf_parser->read_pages( ) ../fpdi_pdf_parser.php:403 95 0.5625 54836864 pdf_parser->pdf_resolve_object( ) ../fpdi_pdf_parser.php:399 96 0.5626 54837544 pdf_parser->pdf_read_value( ) ../pdf_parser.php:577 97 0.5626 54838536 pdf_parser->pdf_read_value( ) ../pdf_parser.php:381 98 0.5626 54838752 pdf_parser->pdf_read_value( ) ../pdf_parser.php:408 99 0.5627 54838752 pdf_parser->pdf_read_token( ) ../pdf_parser.php:476 5. Still unable to view the annotation as student. 6. Too much debugging information in console window with dumping objects
            Hide
            damyon Damyon Wiese added a comment -

            3. Is supposed to work like this. This is the same view that the student sees and is not editable from here (hence no toolbar).

            Show
            damyon Damyon Wiese added a comment - 3. Is supposed to work like this. This is the same view that the student sees and is not editable from here (hence no toolbar).
            Hide
            marina Marina Glancy added a comment -

            3. Makes sense. Maybe then it should say "View annotated PDF" instead of "Launch PDF editor..." ?

            Show
            marina Marina Glancy added a comment - 3. Makes sense. Maybe then it should say "View annotated PDF" instead of "Launch PDF editor..." ?
            Hide
            damyon Damyon Wiese added a comment -

            Fixed pushed for 2, 5 and 6 from last comment. I also changed the text for the link as noted above.

            Still testing with broken pdfs.

            Show
            damyon Damyon Wiese added a comment - Fixed pushed for 2, 5 and 6 from last comment. I also changed the text for the link as noted above. Still testing with broken pdfs.
            Hide
            damyon Damyon Wiese added a comment -

            Fix pushed for 4.

            Show
            damyon Damyon Wiese added a comment - Fix pushed for 4.
            Hide
            damyon Damyon Wiese added a comment -

            Everything should be fixed except for 1. which doesn't happen for me with my broken pdfs. Can you add the broken pdf file you are using as an attachment?

            Thanks!

            Show
            damyon Damyon Wiese added a comment - Everything should be fixed except for 1. which doesn't happen for me with my broken pdfs. Can you add the broken pdf file you are using as an attachment? Thanks!
            Hide
            marina Marina Glancy added a comment -

            I just used an image file with extension changed to pdf. Attaching the screenshot of the error (I sent you the file by email)

            Show
            marina Marina Glancy added a comment - I just used an image file with extension changed to pdf. Attaching the screenshot of the error (I sent you the file by email)
            Hide
            damyon Damyon Wiese added a comment -

            Thanks - testing with this file gives me the same result - but regardless I added a patch that does 2 things:

            1. Makes the pdf library handle this (and the other ones I tested) invalid pdf - by producing a pdf with no pages instead of triggering a stack overflow.

            2. Detect a 0 pages result and show the error message.

            I don't think there is anything outstanding now (except the few I suggested we do in a separate issue).

            Show
            damyon Damyon Wiese added a comment - Thanks - testing with this file gives me the same result - but regardless I added a patch that does 2 things: 1. Makes the pdf library handle this (and the other ones I tested) invalid pdf - by producing a pdf with no pages instead of triggering a stack overflow. 2. Detect a 0 pages result and show the error message. I don't think there is anything outstanding now (except the few I suggested we do in a separate issue).
            Hide
            marina Marina Glancy added a comment -

            thanks Damyon, agree there is nothing outstanding now. Did you want to squash some commits before integration?

            Show
            marina Marina Glancy added a comment - thanks Damyon, agree there is nothing outstanding now. Did you want to squash some commits before integration?
            Hide
            damyon Damyon Wiese added a comment -

            Yes - will squash them now - thanks for the effort in review - this is a huge patch.

            Show
            damyon Damyon Wiese added a comment - Yes - will squash them now - thanks for the effort in review - this is a huge patch.
            Hide
            damyon Damyon Wiese added a comment -

            Rebased, squashed - ready to go!

            Show
            damyon Damyon Wiese added a comment - Rebased, squashed - ready to go!
            Hide
            marina Marina Glancy added a comment -

            Yay! Integrated into master

            Show
            marina Marina Glancy added a comment - Yay! Integrated into master
            Hide
            andyjdavis Andrew Davis added a comment -

            Passing. I have however raised three new MDLs. It works pretty well but there are some quirks to be ironed out.

            Show
            andyjdavis Andrew Davis added a comment - Passing. I have however raised three new MDLs. It works pretty well but there are some quirks to be ironed out.
            Hide
            damyon Damyon Wiese added a comment - - edited

            This is failing unit tests (sorry).

            A fix is available on git pull git://github.com/damyon/moodle.git MDL-42023-master-fix2

            Show
            damyon Damyon Wiese added a comment - - edited This is failing unit tests (sorry). A fix is available on git pull git://github.com/damyon/moodle.git MDL-42023 -master-fix2
            Hide
            damyon Damyon Wiese added a comment -

            Failing for unit tests.

            Show
            damyon Damyon Wiese added a comment - Failing for unit tests.
            Hide
            marina Marina Glancy added a comment -

            fix pulled, back to testing

            Show
            marina Marina Glancy added a comment - fix pulled, back to testing
            Hide
            marina Marina Glancy added a comment -

            unit tests passed. Thanks Damyon

            Show
            marina Marina Glancy added a comment - unit tests passed. Thanks Damyon
            Hide
            poltawski Dan Poltawski added a comment -

            You did it!

            Thanks for your contribution, this change has been merged upstream and is now available on your local git mirror and on download sites shortly.

            Show
            poltawski Dan Poltawski added a comment - You did it! Thanks for your contribution, this change has been merged upstream and is now available on your local git mirror and on download sites shortly.
            Hide
            tsala Helen Foster added a comment -

            Removing qa_test_required label as we now have MDLQA-5724.

            Please shout if the QA test needs amending or adding to at all.

            Show
            tsala Helen Foster added a comment - Removing qa_test_required label as we now have MDLQA-5724 . Please shout if the QA test needs amending or adding to at all.
            Hide
            marycooch Mary Cooch added a comment -

            I have also added (re uploading stamps) MDLQA-5726

            Show
            marycooch Mary Cooch added a comment - I have also added (re uploading stamps) MDLQA-5726
            Hide
            bobcoc Shuming Liu added a comment - - edited

            when i set up the environment(Ghostscript and zlib) to enable this plugin,I found my spawn-fcgi crashed immediately and i got the 502 Bad Gateway,what's wrong with it?
            somebody else had the same problem?
            how to slove it? thanks.

            Show
            bobcoc Shuming Liu added a comment - - edited when i set up the environment(Ghostscript and zlib) to enable this plugin,I found my spawn-fcgi crashed immediately and i got the 502 Bad Gateway,what's wrong with it? somebody else had the same problem? how to slove it? thanks.
            Hide
            william_gibbons William Gibbons added a comment -

            Hello,

            When testing the annotate pdf feature in our moodle 2.6.2 it seems to be working ok but occasionally we have came accross a couple random errors.

            I had a pdf "Voicethread instructions from there site" which I downloaded and uploaded to the moodle assignment. It seems this pdf causes that internal 500 error and our host said something is wrong with that pdf, I am not sure what but the voicethread pdf opens fine with adobe acrobat pro and adobe reader with no problems.

            Another random problem I get from the Instructor and student using the latest google chrome, adobe flash and latest java is occasionally I am getting a TypeError popup when clicking on the annotated pdf but it is random. I have seen it with the latest version of firefox as well. If I close the TypeError windows which lists file, line, stacktrace fields are blank. If I close the secondary popup which says error I am able to view the annotated pdf. Its very random and can be 1-20 times loading the annotated pdf. I was wondering if anyone has experienced this problem and know if there is a fix. This is a new pdf which i created from a word document, saved as a pdf and uploaded as a student to the assignment module.

            Show
            william_gibbons William Gibbons added a comment - Hello, When testing the annotate pdf feature in our moodle 2.6.2 it seems to be working ok but occasionally we have came accross a couple random errors. I had a pdf "Voicethread instructions from there site" which I downloaded and uploaded to the moodle assignment. It seems this pdf causes that internal 500 error and our host said something is wrong with that pdf, I am not sure what but the voicethread pdf opens fine with adobe acrobat pro and adobe reader with no problems. Another random problem I get from the Instructor and student using the latest google chrome, adobe flash and latest java is occasionally I am getting a TypeError popup when clicking on the annotated pdf but it is random. I have seen it with the latest version of firefox as well. If I close the TypeError windows which lists file, line, stacktrace fields are blank. If I close the secondary popup which says error I am able to view the annotated pdf. Its very random and can be 1-20 times loading the annotated pdf. I was wondering if anyone has experienced this problem and know if there is a fix. This is a new pdf which i created from a word document, saved as a pdf and uploaded as a student to the assignment module.
            Hide
            damyon Damyon Wiese added a comment -

            Hi William,

            It is not a good idea to comment on closed issues - they are not often watched. If you have a reproducable error with editpdf please file a new bug. Also editpdf does not use java or flash, so if they are affecting your results, you may have something wrong with your browser.

            Show
            damyon Damyon Wiese added a comment - Hi William, It is not a good idea to comment on closed issues - they are not often watched. If you have a reproducable error with editpdf please file a new bug. Also editpdf does not use java or flash, so if they are affecting your results, you may have something wrong with your browser.

              People

              • Votes:
                0 Vote for this issue
                Watchers:
                10 Start watching this issue

                Dates

                • Created:
                  Updated:
                  Resolved:
                  Fix Release Date:
                  18/Nov/13

                  Agile