Changes between Version 1 and Version 2 of Ticket #18726


Ignore:
Timestamp:
Oct 5, 2015, 9:48:34 AM (6 years ago)
Author:
bill
Comment:

Legend:

Unmodified
Added
Removed
Modified
  • Ticket #18726

    • Property Status changed from new to assigned
    • Property Summary changed from scrollIntoView() suspicious code for IE8 to [regression] scrollIntoView() broken for IE8 quirks and loose mode
    • Property Version changed from 1.10.4 to 1.7.7
    • Property Milestone changed from tbd to 1.7.9
    • Property Owner set to bill
  • Ticket #18726 – Description

    v1 v2  
    1 Both test_scroll.html and window.js are accidentally including IE8, because the `has("trident")` flag is actually truthy for IE8 - IE11, not just IE11.  The changes from https://github.com/dojo/dojo/commit/a832214a342b6e03ad4a9b1669fae71c7938c44b.
     1The changes from https://github.com/dojo/dojo/commit/a832214a342b6e03ad4a9b1669fae71c7938c44b broke `scrollIntoView()` on IE8 for quirks and loose mode, and then masked the error by inadvertently disabling those tests on IE8.
    22
    3 This code from windows.js:
    4 
    5 {{{#!js
    6 if(elPos.x < 0 || !isIE || isIE >= 9 || has("trident")){ elPos.x = 0; } // older IE can have values > 0
    7 if(elPos.y < 0 || !isIE || isIE >= 9 || has("trident")){ elPos.y = 0; }
    8 }}}
    9 
    10 The expression of `isIE >= 9 || has("trident")` is surely a mistake.  But if the current code is working correctly on IE8, then the expression above can be simplified to:
    11 
    12 {{{#!js
    13 elPos.x = elPos.y = 0;
    14 }}}
    15 
    16 This line is also wrong and may be breaking stuff:
    17 
    18 {{{#!js
    19 if(rtl && ((isIE == 8 && !backCompat) || isIE >= 9 || has("trident"))){ s = -s; }
    20 }}}
    21 
    22 Relatedly, this code from test_scroll.html (deleted in master branch but in 1.10 and earlier) is inadvertently (and unnecessarily) skipping most of the tests on IE8:
    23 
    24 {{{#!js
    25 var modes = (has("ie") >= 9 || has("trident")) ? ["_strict"] :
    26       ["_strict", "_quirks", "_loose_rtl", "_quirks_rtl" ];
    27 }}}
    28 
    29 So I'm guessing that https://github.com/dojo/dojo/commit/a832214a342b6e03ad4a9b1669fae71c7938c44b introduced some bugs in IE8 but they are being masked in the regression results.
     3The mistake was because Mark was thinking the `has("trident")` is just for IE11+, but it's actually truthy for IE8 - IE11.