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

Submitted by:  Nutchanon Wetchasit <nachanon>
Submitted on:  Mon 24 Aug 2015 08:09:10 AM UTC  
 
Category: ActionScriptPriority: 5 - Normal
Status: DonePrivacy: Public
Assigned to: Sandro Santilli <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>
Project MemberIn charge of this item.
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>
Project MemberIn charge of this item.
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:

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>

 

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
  • -unavailable- added by strk (Posted a comment)
  • -unavailable- added by nachanon (Submitted the item)
  •  

    Do you think this task is very important?
    If so, you can click here to add your encouragement to it.
    This task has 0 encouragements so far.

    Only logged-in users can vote.

     

    Please enter the title of George Orwell's famous dystopian book (it's a date):

     

     

    Follow 7 latest changes.

    Date Changed By Updated Field Previous Value => Replaced By
    Tue 25 Aug 2015 05:19:41 PM UTCstrkStatusReady For Test=>Done
      Open/ClosedOpen=>Closed
    Tue 25 Aug 2015 05:05:03 PM UTCnachanonAttached File-=>Added 0003_add-textfield-getfontlist-tests.patch, #34725
    Tue 25 Aug 2015 03:31:34 PM UTCnachanonAttached File-=>Added 0002_make-textfield-getfontlist-return-array.patch, #34724
    Mon 24 Aug 2015 11:15:22 AM UTCstrkStatusNone=>Ready For Test
      Assigned toNone=>strk
    Mon 24 Aug 2015 08:09:10 AM UTCnachanonAttached File-=>Added 0001_remove-textfield-getfontlist-this-ptr-check.patch, #34708

    Back to the top


    Powered by Savane 3.1-cleanup1