patchGnash - The GNU Flash player - Patches: patch #8723, Patches to improve the...

 
 

You are not allowed to post comments on this tracker with your current authentication level.

patch #8723: Patches to improve the unimplemented `TextField.getFontList()` stub

Submitter:  Nutchanon Wetchasit <nachanon>
Submitted:  Mon 24 Aug 2015 08:09:10 AM UTC
   
 
Category:  ActionScript Priority:  5 - Normal
Status:  Done Privacy:  Public
Assigned to:  strk Open/Closed:  Closed

Tue 25 Aug 2015 05:19:41 PM UTC, comment #4: 

array return and tests both pushed as commit 2d5a92810cde81303f87961431d9634319f3a652

Sandro Santilli <strk>
Group Member
Tue 25 Aug 2015 05:05:03 PM UTC, comment #3: 

Patch 3 (extra).
See the attached `0003_add-textfield-getfontlist-tests.patch`.

This patch adds five automated tests on how `TextField.getFontList()`
behaves when it was called in various configuration:

  • Called as a static member of TextField class (typical configuration).
  • Called as a function variable.
  • Called as a non-static member of an generic Object.
  • Called with no `this` context, using `Function.call(undefined)`.
  • Called with no `this` context, using `Function.call(null)`.


In all case, `TextField.getFontList()` must return Array to pass.

Not sure if the last test is redundant though; I included it because Gnash
has some issue of `this=null` V.S. `this=undefined` in some place.

Gnash: 0.8.11dev (patched against git e04f9e2 24-Aug-2015 on top of patch #8721)
System: Debian GNU/Linux 7.0 Wheezy i386

(file #34725)

Nutchanon Wetchasit <nachanon>
Tue 25 Aug 2015 03:31:34 PM UTC, comment #2: 

Patch 2 of 2.
See the attached `0002_make-textfield-getfontlist-return-array.patch`.

This patch tries to rectify the problem of `TextField.getFontList()`
returning `undefined` which would cause EFF Panopticlick to fail,
by making it return an empty Array object instead.
(But still printing `UNIMPLEMENTED` log message)

This partially fixes bug #42395.

Note: The code uses `Global_as.createArray()` to create an empty Array,
which is the same way as used in `String.split()`.

The patch has a side effect of allowing proper automated test to be done
on the method. Originally, there was no way to know whether this UNIMPLEMENTED
method call succeeded or failed from ActionScript; both cases returned the
same `undefined` value and exception handler didn't help. With this patch,
successful call returns Array, other values indicate failure.

Automated tests will follow.

Gnash: 0.8.11dev (patched against git e04f9e2 24-Aug-2015)
System: Debian GNU/Linux 7.0 Wheezy i386

(file #34724)

Nutchanon Wetchasit <nachanon>
Mon 24 Aug 2015 11:15:22 AM UTC, comment #1: 

pushed as commit e04f9e25c68ba879e82d206716eae76fc4542cb8
The correct way to test this would be by attaching the getFontList function to an arbitrary object and calling it as a method of that arbitrary objet, like in testsuite/actionscript.all/TextField.as, something like:

var o = {};
o.x = TextField.getFontList;
ret = o.x();
ensure_equals(typeof(ret), whatever);

You'd need to be able to run the resulting SWF with the adobe player to use as reference to spot expected behavior.

Would be a good idea to start with tests as you're sending many patches and it'd be a pity to miss corresponding tests for each...

Sandro Santilli <strk>
Group Member
Mon 24 Aug 2015 08:09:10 AM UTC, original submission:  

Currently Gnash's `TextField.getFontList()` method placeholder code have
a problem that caused ActionScript error when it was called:

ACTIONSCRIPT ERROR: ActionCallMethod: Function requiring gnash::TextField* as 'this' called from gnash::as_object* instance.


This seems to be caused by a `this` pointer check at the method's start
which is incorrect, since `TextField.getFontList()` is a static method;
`this` is not supposed to be available.

Attached patch (patch 1 of 2: `0001_remove-textfield-getfontlist-this-ptr-check.patch`)
tries to rectify the problem by removing the check;
partially fixing bug #42395.

This might need a review before I submit the second patch, since I have seen
two approach of handling this in static method, one in `Math` class
(used by this patch), different way can be seen in `Camera` class.

Gnash: 0.8.11dev (patched against git 88f86a3 19-Aug-2015 on top of patch #8721)
System: Debian GNU/Linux 7.0 Wheezy i386

Nutchanon Wetchasit <nachanon>

 

(Note: upload size limit is set to 16384 kB, after insertion of the required escape characters.)

Attached Files
file #34725:  0003_add-textfield-getfontlist-tests.patch added by nachanon (1KiB - text/x-patch - Patch to add TextField.getFontList() return value tests)
file #34708:  0001_remove-textfield-getfontlist-this-ptr-check.patch added by nachanon (596B - text/x-patch - Patch to remove incorrect `this` pointer check in TextField.getFontList())

 

Depends on the following items: None found

Items that depend on this one: None found

 

Carbon-Copy List
  • -email is unavailable- added by strk (Posted a comment)
  • -email is unavailable- added by nachanon (Submitted the item)
  •  

    There are 0 votes so far. Votes easily highlight which items people would like to see resolved in priority, independently of the priority of the item set by tracker managers.

     

    Follow 7 latest changes.

    Date Changed by Updated Field Previous Value => Replaced by
    2015-08-25 strk StatusReady For Test Done
        Open/ClosedOpen Closed
    2015-08-25 nachanon Attached File- Added 0003_add-textfield-getfontlist-tests.patch, #34725
    2015-08-25 nachanon Attached File- Added 0002_make-textfield-getfontlist-return-array.patch, #34724
    2015-08-24 strk StatusNone Ready For Test
        Assigned toNone strk
    2015-08-24 nachanon Attached File- Added 0001_remove-textfield-getfontlist-this-ptr-check.patch, #34708

    Back to the top

    Powered by Savane 3.13-d3ae.
    Corresponding source code