D10073: [PATCH 1/4] Add JavaScript Event Object handling
This revision was automatically updated to reflect the committed changes. Closed by commit R223:d3a549ca258e: Add JavaScript Event Object handling (authored by aheinecke, committed by aacid). CHANGED PRIOR TO COMMIT https://phabricator.kde.org/D10073?vs=27658=27740#toc REPOSITORY R223 Okular CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D10073?vs=27658=27740 REVISION DETAIL https://phabricator.kde.org/D10073 AFFECTED FILES CMakeLists.txt core/script/event.cpp core/script/event_p.h core/script/executor_kjs.cpp core/script/executor_kjs_p.h core/script/kjs_event.cpp core/script/kjs_event_p.h core/scripter.cpp core/scripter.h To: aheinecke, #okular, aacid Cc: aacid, michaelweghorn, ngraham
D10073: [PATCH 1/4] Add JavaScript Event Object handling
aacid accepted this revision. This revision is now accepted and ready to land. REPOSITORY R223 Okular REVISION DETAIL https://phabricator.kde.org/D10073 To: aheinecke, #okular, aacid Cc: aacid, michaelweghorn, ngraham
D10073: [PATCH 1/4] Add JavaScript Event Object handling
aheinecke updated this revision to Diff 27658. aheinecke added a comment. Removed check for page in field wrapping. REPOSITORY R223 Okular CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D10073?vs=27264=27658 REVISION DETAIL https://phabricator.kde.org/D10073 AFFECTED FILES CMakeLists.txt core/script/event.cpp core/script/event_p.h core/script/executor_kjs.cpp core/script/executor_kjs_p.h core/script/kjs_event.cpp core/script/kjs_event_p.h core/scripter.cpp core/scripter.h To: aheinecke, #okular Cc: aacid, michaelweghorn, ngraham
D10073: [PATCH 1/4] Add JavaScript Event Object handling
aacid added inline comments. INLINE COMMENTS > aheinecke wrote in kjs_field.cpp:217 > It is not assured in kjs_event.cpp eventGetSource and eventGetTarget that the > targetPage / sourcePage is not null. This depends a how the event object is > set up. > > Although I think that sourcePage "should" be set when source is set and > targetPage should be set if target is set there is no guarantee for this and > so I think a check here is warranted. No, no check is warranted, if page is null is because the code is wrong and it should crash so we can fix it not silently do nothing. REPOSITORY R223 Okular REVISION DETAIL https://phabricator.kde.org/D10073 To: aheinecke, #okular Cc: aacid, michaelweghorn, ngraham
D10073: [PATCH 1/4] Add JavaScript Event Object handling
aheinecke added inline comments. INLINE COMMENTS > aacid wrote in kjs_field.cpp:217 > Why the new if? It is not assured in kjs_event.cpp eventGetSource and eventGetTarget that the targetPage / sourcePage is not null. This depends a how the event object is set up. Although I think that sourcePage "should" be set when source is set and targetPage should be set if target is set there is no guarantee for this and so I think a check here is warranted. REPOSITORY R223 Okular REVISION DETAIL https://phabricator.kde.org/D10073 To: aheinecke, #okular Cc: aacid, michaelweghorn, ngraham
D10073: [PATCH 1/4] Add JavaScript Event Object handling
aacid added inline comments.
INLINE COMMENTS
> kjs_field.cpp:217
> KJSObject f = g_fieldProto->constructObject( ctx, field );
> -f.setProperty( ctx, QStringLiteral("page"), page->number() );
> -g_fieldCache->insert( field, page );
> +if (page)
> +{
Why the new if?
REPOSITORY
R223 Okular
REVISION DETAIL
https://phabricator.kde.org/D10073
To: aheinecke, #okular
Cc: aacid, michaelweghorn, ngraham
D10073: [PATCH 1/4] Add JavaScript Event Object handling
aheinecke marked 3 inline comments as done. REPOSITORY R223 Okular REVISION DETAIL https://phabricator.kde.org/D10073 To: aheinecke, #okular Cc: aacid, michaelweghorn, ngraham, simgunz
D10073: [PATCH 1/4] Add JavaScript Event Object handling
aheinecke updated this revision to Diff 27264. aheinecke added a comment. Use non const page refs for source and target page. REPOSITORY R223 Okular CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D10073?vs=26760=27264 REVISION DETAIL https://phabricator.kde.org/D10073 AFFECTED FILES CMakeLists.txt core/script/event.cpp core/script/event_p.h core/script/executor_kjs.cpp core/script/executor_kjs_p.h core/script/kjs_event.cpp core/script/kjs_event_p.h core/script/kjs_field.cpp core/scripter.cpp core/scripter.h To: aheinecke, #okular Cc: aacid, michaelweghorn, ngraham, simgunz
D10073: [PATCH 1/4] Add JavaScript Event Object handling
aheinecke added inline comments. INLINE COMMENTS > aacid wrote in kjs_field.cpp:222 > Or just don't store the page as const pointer? I passed it as const as I only saw API in document to get a const page reference and recalculateForms also used a const reference. But yes I see now that document private can easily access a non const page reference thorugh the page vector. I'll change it. REPOSITORY R223 Okular REVISION DETAIL https://phabricator.kde.org/D10073 To: aheinecke, #okular Cc: aacid, michaelweghorn, ngraham, simgunz
D10073: [PATCH 1/4] Add JavaScript Event Object handling
aacid added inline comments. INLINE COMMENTS > aheinecke wrote in kjs_field.cpp:222 > Indeed I want that now. :-) > > This required an ugly const cast though as the kjs_field API works with a non > const Page pointer. Maybe that could / should be changed? Or just don't store the page as const pointer? REPOSITORY R223 Okular REVISION DETAIL https://phabricator.kde.org/D10073 To: aheinecke, #okular Cc: aacid, michaelweghorn, ngraham
D10073: [PATCH 1/4] Add JavaScript Event Object handling
aheinecke added inline comments. INLINE COMMENTS > aacid wrote in kjs_field.cpp:222 > You really want ot use the verison with the page so refreshPixmaps happens Indeed I want that now. :-) This required an ugly const cast though as the kjs_field API works with a non const Page pointer. Maybe that could / should be changed? REPOSITORY R223 Okular REVISION DETAIL https://phabricator.kde.org/D10073 To: aheinecke, #okular Cc: aacid, michaelweghorn, ngraham
D10073: [PATCH 1/4] Add JavaScript Event Object handling
aheinecke updated this revision to Diff 26760. aheinecke marked an inline comment as done. aheinecke added a comment. Carried the source / target page through to have it available when the kjs_fields are created. REPOSITORY R223 Okular CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D10073?vs=25874=26760 REVISION DETAIL https://phabricator.kde.org/D10073 AFFECTED FILES CMakeLists.txt core/script/event.cpp core/script/event_p.h core/script/executor_kjs.cpp core/script/executor_kjs_p.h core/script/kjs_event.cpp core/script/kjs_event_p.h core/script/kjs_field.cpp core/scripter.cpp core/scripter.h To: aheinecke, #okular Cc: aacid, michaelweghorn, ngraham
D10073: [PATCH 1/4] Add JavaScript Event Object handling
aacid added inline comments.
INLINE COMMENTS
> kjs_field.cpp:222
>
> +KJSObject JSField::wrapField( KJSContext *ctx, FormField *field)
> +{
You really want ot use the verison with the page so refreshPixmaps happens
REPOSITORY
R223 Okular
REVISION DETAIL
https://phabricator.kde.org/D10073
To: aheinecke, #okular
Cc: aacid, michaelweghorn, ngraham
D10073: [PATCH 1/4] Add JavaScript Event Object handling
aheinecke added a task: T7805: Okular: Add support for JavaScript AFSimple_Calculate and textfield content calculation. REPOSITORY R223 Okular REVISION DETAIL https://phabricator.kde.org/D10073 To: aheinecke, #okular Cc: michaelweghorn, ngraham, aacid
D10073: [PATCH 1/4] Add JavaScript Event Object handling
aheinecke created this revision. aheinecke added a reviewer: Okular. aheinecke added a project: Okular. aheinecke requested review of this revision. REVISION SUMMARY This adds a new data object "Event" that can be used to carry information in and out of JavaScript execution contexts. The Event Object is defined in the Adobe JavaScript scripting reference. The implementation now adds handling for the FieldCalculate Event. It should be extensible enough so that in the future more events could be supported. REPOSITORY R223 Okular REVISION DETAIL https://phabricator.kde.org/D10073 AFFECTED FILES CMakeLists.txt core/script/event.cpp core/script/event_p.h core/script/executor_kjs.cpp core/script/executor_kjs_p.h core/script/kjs_event.cpp core/script/kjs_event_p.h core/script/kjs_field.cpp core/script/kjs_field_p.h core/scripter.cpp core/scripter.h To: aheinecke, #okular Cc: michaelweghorn, ngraham, aacid
