Bug 14383 - [S60] Problem with CPluginSkin::PluginScriptableObject TSW ID : MKUI-75UDR2
Summary: [S60] Problem with CPluginSkin::PluginScriptableObject TSW ID : MKUI-75UDR2
Status: CLOSED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Plug-ins (show other bugs)
Version: 523.x (Safari 3)
Hardware: S60 Emulator S60 3rd edition
: P2 Normal
Assignee: Nobody
URL: http://waplabdc.nokia-boston.com/brow...
Keywords:
Depends on:
Blocks:
 
Reported: 2007-06-25 07:04 PDT by Mahesh Kulkarni
Modified: 2008-04-09 11:39 PDT (History)
3 users (show)

See Also:


Attachments
Problem with CPluginSkin::PluginScriptableObject (1.26 KB, patch)
2007-06-25 07:09 PDT, Mahesh Kulkarni
Sachin.Padma: review-
Details | Formatted Diff | Diff
3.1 patch. added check for widget and flash (2.44 KB, patch)
2007-06-27 03:07 PDT, Mahesh Kulkarni
no flags Details | Formatted Diff | Diff
CCB patch. Added scriptability for selected plugins (1.96 KB, patch)
2007-06-27 03:31 PDT, Mahesh Kulkarni
no flags Details | Formatted Diff | Diff
3.1m Patch for scriptability check (5.85 KB, patch)
2007-07-11 23:08 PDT, Mahesh Kulkarni
Sachin.Padma: review-
Details | Formatted Diff | Diff
3.1 patch - Updated as per review comments (6.14 KB, patch)
2007-07-23 06:27 PDT, Mahesh Kulkarni
Sachin.Padma: review-
Details | Formatted Diff | Diff
3.1 patch - removed tab in one place. (6.34 KB, patch)
2007-07-26 22:57 PDT, Mahesh Kulkarni
Sachin.Padma: review-
Details | Formatted Diff | Diff
3.1 patch - Attached changeLog (7.03 KB, patch)
2007-07-30 02:29 PDT, Mahesh Kulkarni
Sachin.Padma: review-
Details | Formatted Diff | Diff
3.1 patch (7.03 KB, patch)
2007-08-02 04:40 PDT, Mahesh Kulkarni
Sachin.Padma: review-
Details | Formatted Diff | Diff
3.1 patch - WITH LATEST CODE (7.12 KB, patch)
2007-08-09 06:13 PDT, Mahesh Kulkarni
Sachin.Padma: review+
Details | Formatted Diff | Diff
3.1 patch - Build break fix (1.58 KB, patch)
2007-08-09 23:50 PDT, Mahesh Kulkarni
Sachin.Padma: review+
Details | Formatted Diff | Diff
3.1 patch (1.11 KB, patch)
2007-08-28 06:51 PDT, Mahesh Kulkarni
Sachin.Padma: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Mahesh Kulkarni 2007-06-25 07:04:59 PDT
_DESC:

CPluginSkin::PluginScriptableObject function in pluginSkin.cpp checks for widgets which makes only widget take use of scriptability. Where as flash can also be a scriptable. 

_PROBLEM:

AS-JS interaction of flash wont work as currently widgets are allowed t ouse scriptable object. 

_SOLUTION:

Removing widget only check from CPluginSkin::PluginScriptableObject.

_IMPACT:

WebKit\plugin
Comment 1 Mahesh Kulkarni 2007-06-25 07:09:10 PDT
Created attachment 15217 [details]
Problem with CPluginSkin::PluginScriptableObject
Comment 2 Mahesh Kulkarni 2007-06-27 03:07:01 PDT
Created attachment 15269 [details]
3.1 patch. added check for widget and flash

This is 3.1 patch. Removed widget only check from pluginSkin.cpp and added check for flash/widget check in pluginWin.cpp.
Comment 3 Mahesh Kulkarni 2007-06-27 03:31:02 PDT
Created attachment 15270 [details]
CCB patch. Added scriptability for selected plugins
Comment 4 Mahesh Kulkarni 2007-07-11 22:56:12 PDT
Comment on attachment 15269 [details]
3.1 patch. added check for widget and flash

Discarding these patch's. As sriram has suggested to use repository and uid of plugin to check if scriptability is allowed.
Comment 5 Mahesh Kulkarni 2007-07-11 23:08:58 PDT
Created attachment 15482 [details]
3.1m Patch for scriptability check

According to Sriram/AnttiH/Yael comments, checking for scriptability of plugins is specific to plugin UID which is compared with the one in browser/webui repository. 

1) Added KPluginScriptabilityAllowed in KCRUidBrowser and KCRUidWebUi repository.
2) KPluginScriptabilityAllowed can have more than one plugin's UID's
3) Added Api PluginInfo->Uid() to get plugin UID
4) Added Api PluginWin->GetPluginUid() to access plugin UID in pluginSkin
5) Added Api pluginSkin->IsPluginScriptableL checks if KPluginScriptabilityAllowed present in the repository. 

Note: I have not attached xls files. Because I am not sure should we add or how to add xls files here. And even BrowserUiInternalCRKeys.h file which is not part of OssWebEngine.
Comment 6 Sachin Padma 2007-07-19 09:02:10 PDT
Comment on attachment 15482 [details]
3.1m Patch for scriptability check

You cannot use leave to return a value from function. Please leave only if there is a critical error like memory allocation failure etc. Otherwise return a value. No need for uidPtr, just pass in Des()
Comment 7 Mahesh Kulkarni 2007-07-23 06:27:49 PDT
Created attachment 15648 [details]
3.1 patch - Updated as per review comments

Changes made as per comments,

1) Made function return value. Leave happens only on special cases as per comments from sachin. 

2) uidPtr can not be removed, as repository->Get takes reference pointer and by trying to pass Des() of uid giving compilation error.
Comment 8 Sachin Padma 2007-07-26 12:41:52 PDT
Comment on attachment 15648 [details]
3.1 patch - Updated as per review comments

Please make sure there are no tabs.
Comment 9 Mahesh Kulkarni 2007-07-26 22:57:18 PDT
Created attachment 15696 [details]
3.1 patch - removed tab in one place.

Removed one tab in PluginHandler.h. Will make sure for no tabs in patches in future.
Comment 10 Sachin Padma 2007-07-27 07:24:18 PDT
Comment on attachment 15696 [details]
3.1 patch - removed tab in one place.

r=me
Comment 11 Sachin Padma 2007-07-27 07:24:51 PDT
Comment on attachment 15696 [details]
3.1 patch - removed tab in one place.

Please attach changelog
Comment 12 Mahesh Kulkarni 2007-07-30 02:29:58 PDT
Created attachment 15740 [details]
3.1 patch - Attached changeLog

Attached change log.
Comment 13 Sachin Padma 2007-08-01 13:07:56 PDT
Comment on attachment 15740 [details]
3.1 patch - Attached changeLog

Please remove the tabs.
Comment 14 Mahesh Kulkarni 2007-08-02 04:40:00 PDT
Created attachment 15805 [details]
3.1 patch

Removed Tab. Which software do you use to find tabs?
Comment 15 Mahesh Kulkarni 2007-08-07 03:12:42 PDT
Added TSW error report for this bug
Comment 16 Sachin Padma 2007-08-08 14:59:00 PDT
Comment on attachment 15805 [details]
3.1 patch

Patch does not apply.

patching file `S60WebUi/WebUi/inc/WebUiCRKeys.h'
patching file `WebKit/ChangeLog'
Hunk #1 succeeded at 1 with fuzz 3.
patching file `WebKit/Plugin/inc/PluginHandler.h'
patching file `WebKit/Plugin/inc/PluginSkin.h'
Hunk #1 FAILED at 378.
1 out of 1 hunk FAILED -- saving rejects to WebKit/Plugin/inc/PluginSkin.h.rej
patching file `WebKit/Plugin/inc/PluginWin.h'
patching file `WebKit/Plugin/src/PluginSkin.cpp'
Reversed (or previously applied) patch detected!  Assume -R? [n]
Apply anyway? [n]
Skipping patch.
3 out of 3 hunks ignored -- saving rejects to WebKit/Plugin/src/PluginSkin.cpp.r
ej
patching file `WebKit/Plugin/src/PluginWin.cpp'
Comment 17 Mahesh Kulkarni 2007-08-09 06:13:59 PDT
Created attachment 15880 [details]
3.1 patch - WITH LATEST CODE

Patched with latest 3.1 code. I also need to modify xls's for this bug as i have to update repository. Please let me know how do i go about getting xls's to link with this patch. 

Thanks
Comment 18 Sachin Padma 2007-08-09 08:30:33 PDT
Comment on attachment 15880 [details]
3.1 patch - WITH LATEST CODE

r=me
Comment 19 Sachin Padma 2007-08-09 08:33:13 PDT
laned in r24955
Comment 20 Mahesh Kulkarni 2007-08-09 23:50:10 PDT
Created attachment 15895 [details]
3.1 patch - Build break fix

Sorry for the build break. As commented above, I was not sure if we can attach xls files in patch. Thinking xls's can not be compared with any tool. I will take care of these issues in future.
Comment 21 Sachin Padma 2007-08-10 12:01:33 PDT
Comment on attachment 15895 [details]
3.1 patch - Build break fix

r=me
Comment 22 Mahesh Kulkarni 2007-08-28 06:51:33 PDT
Created attachment 16143 [details]
3.1 patch

Wrong check in CPluginSkin::PluginScriptableObject for same TSW error.
Comment 23 Sachin Padma 2007-08-31 11:32:45 PDT
Comment on attachment 16143 [details]
3.1 patch

r=me
Comment 24 Bradley Morrison 2007-09-04 12:12:41 PDT
do both attachment 16143 [details] & 15895 need to get landed?
Comment 25 Bradley Morrison 2008-04-09 11:39:41 PDT
Bulk closing of all s60 platform bugs. 

Sorry for the noise!