You are viewing a plain text version of this content. The canonical link for it is here.

Posted to commits@weex.apache.org by dechaos <gi...@git.apache.org> on 2018/02/12 09:13:37 UTC

[GitHub] incubator-weex pull request #1028: [WEEX-217][iOS] fix:WXTransform should no...

GitHub user dechaos opened a pull request:

    https://github.com/apache/incubator-weex/pull/1028

    [WEEX-217][iOS] fix:WXTransform should not crash while parsing 'translate(0)' on iOS

    According to the latest published W3C specification about CSS Transforms:
    
    translate() = translate( <length-percentage> [, <length-percentage> ]? )
    specifies a 2D translation by the vector [tx, ty], where tx is the first translation-value parameter and ty is the optional second translation-value parameter. If <ty> is not provided, ty has zero as a value.
    
    , translate(tx) is equal to translate(tx, 0). In the previous version of Weex, we removed the array length check in method [WXTransform parseTranslate:]. if the parser encounters a 'translate(0)', the array contains only one single value inside, parseTranslate will fetch the element at index 1 from the array, which causes a typical out-of-bounds exception, and lead to an app crash eventually.
    
    We should add the array length check back to the method to avoid the crash in the above case.
    
    As we known in many js packing procedures, 'translate(x, 0)' will be compressed/minified to the form 'translate(x)’, Weex should avoid such inconsistent implementations and conforms the W3C specifications.
    
    Bug: 217
    
    First of all, thank you for your contribution! 
    
    Please makes sure that these checkboxes are checked before submitting your PR, thank you!
    
    <!-- 
    How to use github markdown?
    ### checkable example
    - [x] checked
    - [ ] not checked
    More github Markdown info to see https://guides.github.com/features/mastering-markdown/
    -->
    
    CheckList:
    
    **if** *isBugFix* **:**
    
      * [ ] Make sure that you add at least one unit test for the bug which you had fixed.
    
    **elif** *isNewFeature* **:**
    
      * [ ] Update API docs for the component.
      * [ ] Update/Add demo to demonstrate new feature.
      * [ ] Update test scripts for the feature.
      * [ ] Add unit tests for the feature.


You can merge this pull request into a Git repository by running:

    $ git pull https://github.com/dechaos/incubator-weex fix-translate-crash

Alternatively you can review and apply these changes as the patch at:

    https://github.com/apache/incubator-weex/pull/1028.patch

To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:

    This closes #1028
    
----
commit 12bff01244ef2bcf3796dc3645ec157de5db6e34
Author: 如展 <ga...@...>
Date:   2018-02-12T09:07:16Z

    [WEEX-217][iOS] fix:WXTransform should not crash while parsing 'translate(0)' on iOS
    
    According to the latest published W3C specification about CSS Transforms:
    
    translate() = translate( <length-percentage> [, <length-percentage> ]? )
    specifies a 2D translation by the vector [tx, ty], where tx is the first translation-value parameter and ty is the optional second translation-value parameter. If <ty> is not provided, ty has zero as a value.
    
    , translate(tx) is equal to translate(tx, 0). In the previous version of Weex, we removed the array length check in method [WXTransform parseTranslate:]. if the parser encounters a 'translate(0)', the array contains only one single value inside, parseTranslate will fetch the element at index 1 from the array, which causes a typical out-of-bounds exception, and lead to an app crash eventually.
    
    We should add the array length check back to the method to avoid the crash in the above case.
    
    As we known in many js packing procedures, 'translate(x, 0)' will be compressed/minified to the form 'translate(x)’, Weex should avoid such inconsistent implementations and conforms the W3C specifications.
    
    Bug: 217

----


---

[GitHub] incubator-weex issue #1028: [WEEX-217][iOS] fix:WXTransform should not crash...

Posted by acton393 <gi...@git.apache.org>.

Github user acton393 commented on the issue:

    https://github.com/apache/incubator-weex/pull/1028
  
    @dechaos  thank you for you contribution, and the file change looks good to me,  a very specific explanation at the same time for CSS of W3C rules, let's make it more standard.


---

[GitHub] incubator-weex issue #1028: [WEEX-217][iOS] fix:WXTransform should not crash...

Posted by weex-bot <gi...@git.apache.org>.

Github user weex-bot commented on the issue:

    https://github.com/apache/incubator-weex/pull/1028
  
    
    <!--
      0 failure: 
      0 warning: 
      2 messages
      
    -->
    
    
    
    <table>
      <thead>
        <tr>
          <th width="50"></th>
          <th width="100%" data-danger-table="true">Messages</th>
        </tr>
      </thead>
      <tbody><tr>
          <td>:book:</td>
          <td>has no android file changed.</td>
        </tr>
      
    <tr>
          <td>:book:</td>
          <td>android test finished.</td>
        </tr>
      </tbody>
    </table>
    
    
    <p align="right">
      Generated by :no_entry_sign: <a href="http://github.com/danger/danger-js/">dangerJS</a>
    </p>



---

[GitHub] incubator-weex pull request #1028: [WEEX-217][iOS] fix:WXTransform should no...

Posted by asfgit <gi...@git.apache.org>.

Github user asfgit closed the pull request at:

    https://github.com/apache/incubator-weex/pull/1028


---