Work around Chrome bug?

classic Classic list List threaded Threaded
5 messages Options
Reply | Threaded
Open this post in threaded view
|

Work around Chrome bug?

Harbs
I discovered a bug in Blink (Chrome and Opera).[1]

The workaround for this is to set the width and height of svg elements to round numbers (using Math.ceil seems to generally be a good way of doing this). I have implemented the workaround in my app and it works well. I’m debating on whether this is something that should go in framework code.

Reasons to add it:
1. It will make SVG components display correctly in all browsers.
2. Client developers do not need to be aware of this issue.

Reasons not to:
1. It’s a bug which I believe will only present itself in cases where there is transforms applied to the elements.
2. It’s hopefully a temporary problem until it’s fixed in Blink.
3. It’s “just in case” code that’s only needed for Chrome and Opera and is only needed for certain types of svg hierarchies.
4. It’s relatively easy to work around in client code.
5. I’m pretty sure that elements which have a scale transformation will display incorrectly if the width and height values are rounded.

To me, reason number 5 is a pretty compelling reason to NOT fix this in the framework. I do think we should document this bug with the workaround somewhere though.

Thoughts?

Harbs

[1]https://bugs.chromium.org/p/chromium/issues/detail?id=757196
Reply | Threaded
Open this post in threaded view
|

Re: Work around Chrome bug?

Alex Harui-2
I agree that we shouldn't make Math.ceil the default code path.  What does
the change look like?  Could it be post-processed onto the JS output when
somebody needs it?  I didn't think Opera was one of our target browsers.

-Alex

On 8/20/17, 1:23 AM, "Harbs" <[hidden email]> wrote:

>I discovered a bug in Blink (Chrome and Opera).[1]
>
>The workaround for this is to set the width and height of svg elements to
>round numbers (using Math.ceil seems to generally be a good way of doing
>this). I have implemented the workaround in my app and it works well. I’m
>debating on whether this is something that should go in framework code.
>
>Reasons to add it:
>1. It will make SVG components display correctly in all browsers.
>2. Client developers do not need to be aware of this issue.
>
>Reasons not to:
>1. It’s a bug which I believe will only present itself in cases where
>there is transforms applied to the elements.
>2. It’s hopefully a temporary problem until it’s fixed in Blink.
>3. It’s “just in case” code that’s only needed for Chrome and Opera and
>is only needed for certain types of svg hierarchies.
>4. It’s relatively easy to work around in client code.
>5. I’m pretty sure that elements which have a scale transformation will
>display incorrectly if the width and height values are rounded.
>
>To me, reason number 5 is a pretty compelling reason to NOT fix this in
>the framework. I do think we should document this bug with the workaround
>somewhere though.
>
>Thoughts?
>
>Harbs
>
>[1]https://na01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fbugs.c
>hromium.org%2Fp%2Fchromium%2Fissues%2Fdetail%3Fid%3D757196&data=02%7C01%7C
>%7C6de72659db7c4afeeb1f08d4e7a4b5ea%7Cfa7b1b5a7b34438794aed2c178decee1%7C0
>%7C0%7C636388141989832483&sdata=9eYrcJ4kvQITIot%2BibogmJhiPaV%2FBsVt1KCM1t
>5fO%2BU%3D&reserved=0

Reply | Threaded
Open this post in threaded view
|

Re: Re: Work around Chrome bug?

after24
In reply to this post by Harbs
Bonjour,

En congés jusqu'au 4 septembre, je prendrais connaissance de votre message à mon retour.

Cordialement,

Vincent/AFTER24



Reply | Threaded
Open this post in threaded view
|

Re: Work around Chrome bug?

Harbs
In reply to this post by Alex Harui-2
Modern versions of Opera use Blink and its support is probably pretty analogous to Chrome.

The fix looks like this:
                override public function set width(value:Number):void{
                        contentContainer.width = value;
                        // needed until https://bugs.chromium.org/p/chromium/issues/detail?id=757196 is fixed.
                        contentContainer.positioner.style.width = "" + Math.ceil(value) + "px";
                }

                override public function set height(value:Number):void{
                        contentContainer.height = value;
                        // needed until https://bugs.chromium.org/p/chromium/issues/detail?id=757196 is fixed.
                        contentContainer.positioner.style.height = "" + Math.ceil(value) + "px";
                }

I’m wrapping an SVG in a div and setting the size on the svg rather than the div. If someone was fixing it on the actual component, it would probably look something like this:

                override public function set width(value:Number):void{
                        super.width = value;
                        // needed until https://bugs.chromium.org/p/chromium/issues/detail?id=757196 is fixed.
                        positioner.style.width = "" + Math.ceil(value) + "px";
                }

                override public function set height(value:Number):void{
                        super.height = value;
                        // needed until https://bugs.chromium.org/p/chromium/issues/detail?id=757196 is fixed.
                        positioner.style.height = "" + Math.ceil(value) + "px";
                }


> On Aug 21, 2017, at 8:56 AM, Alex Harui <[hidden email]> wrote:
>
> I agree that we shouldn't make Math.ceil the default code path.  What does
> the change look like?  Could it be post-processed onto the JS output when
> somebody needs it?  I didn't think Opera was one of our target browsers.
>
> -Alex
>
> On 8/20/17, 1:23 AM, "Harbs" <[hidden email]> wrote:
>
>> I discovered a bug in Blink (Chrome and Opera).[1]
>>
>> The workaround for this is to set the width and height of svg elements to
>> round numbers (using Math.ceil seems to generally be a good way of doing
>> this). I have implemented the workaround in my app and it works well. I’m
>> debating on whether this is something that should go in framework code.
>>
>> Reasons to add it:
>> 1. It will make SVG components display correctly in all browsers.
>> 2. Client developers do not need to be aware of this issue.
>>
>> Reasons not to:
>> 1. It’s a bug which I believe will only present itself in cases where
>> there is transforms applied to the elements.
>> 2. It’s hopefully a temporary problem until it’s fixed in Blink.
>> 3. It’s “just in case” code that’s only needed for Chrome and Opera and
>> is only needed for certain types of svg hierarchies.
>> 4. It’s relatively easy to work around in client code.
>> 5. I’m pretty sure that elements which have a scale transformation will
>> display incorrectly if the width and height values are rounded.
>>
>> To me, reason number 5 is a pretty compelling reason to NOT fix this in
>> the framework. I do think we should document this bug with the workaround
>> somewhere though.
>>
>> Thoughts?
>>
>> Harbs
>>
>> [1]https://na01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fbugs.c
>> hromium.org%2Fp%2Fchromium%2Fissues%2Fdetail%3Fid%3D757196&data=02%7C01%7C
>> %7C6de72659db7c4afeeb1f08d4e7a4b5ea%7Cfa7b1b5a7b34438794aed2c178decee1%7C0
>> %7C0%7C636388141989832483&sdata=9eYrcJ4kvQITIot%2BibogmJhiPaV%2FBsVt1KCM1t
>> 5fO%2BU%3D&reserved=0
>

Reply | Threaded
Open this post in threaded view
|

Re: Work around Chrome bug?

Alex Harui-2
How many JS files are affected?  If it is only a few and you think it is a
bug that will be fixed, then it is probably better to just monkey patch
your JS files.  It would be a good test case for making sure we've made it
easy enough to do such a thing.  You should be able to set up a folder
structure with the relevant package paths and then point to it with the
-sdk-js-lib option.

HTH,
-Alex

On 8/21/17, 12:39 AM, "Harbs" <[hidden email]> wrote:

>Modern versions of Opera use Blink and its support is probably pretty
>analogous to Chrome.
>
>The fix looks like this:
> override public function set width(value:Number):void{
> contentContainer.width = value;
> // needed until
>https://na01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fbugs.chro
>mium.org%2Fp%2Fchromium%2Fissues%2Fdetail%3Fid%3D757196&data=02%7C01%7C%7C
>ffb2d3cdfc694d86db0708d4e867d2c0%7Cfa7b1b5a7b34438794aed2c178decee1%7C0%7C
>0%7C636388979991056998&sdata=OsmlLmMJwAtD7xB2tdl1d69eKUp6T3vQZ9BaiDnnyho%3
>D&reserved=0 is fixed.
> contentContainer.positioner.style.width = "" + Math.ceil(value) + "px";
> }
>
> override public function set height(value:Number):void{
> contentContainer.height = value;
> // needed until
>https://na01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fbugs.chro
>mium.org%2Fp%2Fchromium%2Fissues%2Fdetail%3Fid%3D757196&data=02%7C01%7C%7C
>ffb2d3cdfc694d86db0708d4e867d2c0%7Cfa7b1b5a7b34438794aed2c178decee1%7C0%7C
>0%7C636388979991056998&sdata=OsmlLmMJwAtD7xB2tdl1d69eKUp6T3vQZ9BaiDnnyho%3
>D&reserved=0 is fixed.
> contentContainer.positioner.style.height = "" + Math.ceil(value) +
>"px";
> }
>
>I’m wrapping an SVG in a div and setting the size on the svg rather than
>the div. If someone was fixing it on the actual component, it would
>probably look something like this:
>
> override public function set width(value:Number):void{
> super.width = value;
> // needed until
>https://na01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fbugs.chro
>mium.org%2Fp%2Fchromium%2Fissues%2Fdetail%3Fid%3D757196&data=02%7C01%7C%7C
>ffb2d3cdfc694d86db0708d4e867d2c0%7Cfa7b1b5a7b34438794aed2c178decee1%7C0%7C
>0%7C636388979991056998&sdata=OsmlLmMJwAtD7xB2tdl1d69eKUp6T3vQZ9BaiDnnyho%3
>D&reserved=0 is fixed.
> positioner.style.width = "" + Math.ceil(value) + "px";
> }
>
> override public function set height(value:Number):void{
> super.height = value;
> // needed until
>https://na01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fbugs.chro
>mium.org%2Fp%2Fchromium%2Fissues%2Fdetail%3Fid%3D757196&data=02%7C01%7C%7C
>ffb2d3cdfc694d86db0708d4e867d2c0%7Cfa7b1b5a7b34438794aed2c178decee1%7C0%7C
>0%7C636388979991056998&sdata=OsmlLmMJwAtD7xB2tdl1d69eKUp6T3vQZ9BaiDnnyho%3
>D&reserved=0 is fixed.
> positioner.style.height = "" + Math.ceil(value) + "px";
> }
>
>
>> On Aug 21, 2017, at 8:56 AM, Alex Harui <[hidden email]>
>>wrote:
>>
>> I agree that we shouldn't make Math.ceil the default code path.  What
>>does
>> the change look like?  Could it be post-processed onto the JS output
>>when
>> somebody needs it?  I didn't think Opera was one of our target browsers.
>>
>> -Alex
>>
>> On 8/20/17, 1:23 AM, "Harbs" <[hidden email]> wrote:
>>
>>> I discovered a bug in Blink (Chrome and Opera).[1]
>>>
>>> The workaround for this is to set the width and height of svg elements
>>>to
>>> round numbers (using Math.ceil seems to generally be a good way of
>>>doing
>>> this). I have implemented the workaround in my app and it works well.
>>>I’m
>>> debating on whether this is something that should go in framework code.
>>>
>>> Reasons to add it:
>>> 1. It will make SVG components display correctly in all browsers.
>>> 2. Client developers do not need to be aware of this issue.
>>>
>>> Reasons not to:
>>> 1. It’s a bug which I believe will only present itself in cases where
>>> there is transforms applied to the elements.
>>> 2. It’s hopefully a temporary problem until it’s fixed in Blink.
>>> 3. It’s “just in case” code that’s only needed for Chrome and Opera and
>>> is only needed for certain types of svg hierarchies.
>>> 4. It’s relatively easy to work around in client code.
>>> 5. I’m pretty sure that elements which have a scale transformation will
>>> display incorrectly if the width and height values are rounded.
>>>
>>> To me, reason number 5 is a pretty compelling reason to NOT fix this in
>>> the framework. I do think we should document this bug with the
>>>workaround
>>> somewhere though.
>>>
>>> Thoughts?
>>>
>>> Harbs
>>>
>>>
>>>[1]https://na01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fbugs
>>>.c
>>>
>>>hromium.org%2Fp%2Fchromium%2Fissues%2Fdetail%3Fid%3D757196&data=02%7C01%
>>>7C
>>>
>>>%7C6de72659db7c4afeeb1f08d4e7a4b5ea%7Cfa7b1b5a7b34438794aed2c178decee1%7
>>>C0
>>>
>>>%7C0%7C636388141989832483&sdata=9eYrcJ4kvQITIot%2BibogmJhiPaV%2FBsVt1KCM
>>>1t
>>> 5fO%2BU%3D&reserved=0
>>
>