Tryton - Issues

 

Issue7195

Title Remove svg blob spefic management for firefox
Priority feature Status testing
Superseder Use Material icons
View: 474
Nosy List ced, pokoli, reviewbot
Type feature request Components sao
Assigned To ced Keywords review
Reviews 44721002, 60291002
View: 44721002, 60291002

Created on 2018-03-06.16:59:42 by pokoli, last changed by reviewbot.

Messages
review60291002 updated at https://codereview.tryton.org/60291002/#ps50001
New review60291002 at https://codereview.tryton.org/60291002/#ps40001
msg42621 (view) Author: [hidden] (ced) (Tryton committer) (Tryton translator) Date: 2018-08-03.01:05:33
To implement Material icons for issue474, I needed to remove this code which is then simpler.
msg40436 (view) Author: [hidden] (pokoli) (Tryton committer) (Tryton translator) Date: 2018-04-27.14:03:59
It's strange because I see correctly all the icons on the menu. I confirm that the error is only shown when using a custom set of icons but is strange as I reproduced it with two different sets of icons.
review46411002 updated at https://codereview.tryton.org/46411002/#ps1
msg40433 (view) Author: [hidden] (ced) (Tryton committer) (Tryton translator) Date: 2018-04-27.12:57:11
I do not agree with the proposed fix. The documentation states that the callback should receive a Blob so there is no reason to test if it is null.
I guess it is because you have a wrong icon definition which returns an None for the icon. But I do not think the clients should be fixed against bad data.
msg40432 (view) Author: [hidden] (pokoli) (Tryton committer) (Tryton translator) Date: 2018-04-27.12:36:40
The problem is that the blob data is empty. Here is review46411002 which tests the emptyness to avoid the error.
msg38853 (view) Author: [hidden] (ced) (Tryton committer) (Tryton translator) Date: 2018-03-07.14:36:33
According to the documentation we should receive a Blob: https://developer.mozilla.org/en-US/docs/Web/API/HTMLCanvasElement/toBlob
But maybe it is because you get an empty (or invalid) image.
msg38851 (view) Author: [hidden] (pokoli) (Tryton committer) (Tryton translator) Date: 2018-03-07.14:09:37
Here is the non-minified traceback:

TypeError: Argument 1 is not valid for any of the 1-argument overloads of URL.createObjectURL.
tryton-sao.js:17292:48
register_icon/this.register_prm</</</image.onload</<
http://localhost:8000/dist/tryton-sao.js:17292:48
<anonymous> self-hosted:951:17

The problem is that blob is undefined on this line: 

http://hg.tryton.org/sao/file/e15594edbcc5/src/common.js#l2576
msg38850 (view) Author: [hidden] (ced) (Tryton committer) (Tryton translator) Date: 2018-03-07.14:06:05
Please provide a non-minified traceback.
msg38849 (view) Author: [hidden] (pokoli) (Tryton committer) (Tryton translator) Date: 2018-03-07.13:04:47
On my browser this code generates the following console.error:

TypeError: Argument 1 is not valid for any of the 1-argument overloads of URL.createObjectURL.
  at register_icon/this.register_prm</</</d.onload</<(/node_modules/tryton-sao/dist/tryton-sao.min.js:13:2798)

Which makes me doubt if it works correctly.
msg38847 (view) Author: [hidden] (ced) (Tryton committer) (Tryton translator) Date: 2018-03-07.10:41:02
OK but do we still want to cut off more than 5% of our audience? while we have a working fix.
msg38846 (view) Author: [hidden] (pokoli) (Tryton committer) (Tryton translator) Date: 2018-03-07.10:23:58
El 07/03/18 a les 10:10, C├ędric Krier ha escrit:
> On 2018-03-07 09:34, Sergi Almacellas Abellana wrote:
>> Sergi Almacellas Abellana <sergi@koolpi.com> added the comment:
>>
>>> I'm not sure if we should be so fast at dropping older browsers.
>> For example, jQuery only supports current and last version of common browsers:
>>
>> https://jquery.com/browser-support/
> OK, but what is the current browser on common LTS Linux distro? 

Ubuntu provides the latest firefox version for it's LTS versions:

https://packages.ubuntu.com/trusty/firefox

Fedora also provides the last firefox version for all versions:

https://apps.fedoraproject.org/packages/firefox

Debian uses an Extended Support Relase (don't know what this mean), but the latest stable version is based on firefox 52:

https://packages.debian.org/stretch/firefox-esr

> Or on iOS or Android?

I've got the latest firefox version on Android. Applications are updated automatically if installed from play store (which is what most users do).
msg38845 (view) Author: [hidden] (ced) (Tryton committer) (Tryton translator) Date: 2018-03-07.10:10:12
On 2018-03-07 09:34, Sergi Almacellas Abellana wrote:
> 
> Sergi Almacellas Abellana <sergi@koolpi.com> added the comment:
> 
> > I'm not sure if we should be so fast at dropping older browsers.
> 
> For example, jQuery only supports current and last version of common browsers:
> 
> https://jquery.com/browser-support/

OK, but what is the current browser on common LTS Linux distro? Or on
iOS or Android?

> > It is very recent that browsers are publishing very fast new releases.
> > The statistics of tryton.org shows that there are still 5.7% of Firefox 40.1
> 
> So when do you propose to remove them?

I do not know but this should be carefully planned.
msg38844 (view) Author: [hidden] (pokoli) (Tryton committer) (Tryton translator) Date: 2018-03-07.09:34:30
> I'm not sure if we should be so fast at dropping older browsers.

For example, jQuery only supports current and last version of common browsers:

https://jquery.com/browser-support/


> It is very recent that browsers are publishing very fast new releases.
> The statistics of tryton.org shows that there are still 5.7% of Firefox 40.1

So when do you propose to remove them?
msg38834 (view) Author: [hidden] (ced) (Tryton committer) (Tryton translator) Date: 2018-03-06.17:25:57
I'm not sure if we should be so fast at dropping older browsers.
It is very recent that browsers are publishing very fast new releases.
The statistics of tryton.org shows that there are still 5.7% of Firefox 40.1
review44721002 updated at https://codereview.tryton.org/44721002/#ps1
msg38831 (view) Author: [hidden] (pokoli) (Tryton committer) (Tryton translator) Date: 2018-03-06.17:14:26
> When was released firefox 50? How long is a release supported?
Firefox 50 was released on November 15, 2016 [1] and its end of life was January 24, 2017 [2]. 

IIUC firefox only supports the last major version. 

[1] https://www.mozilla.org/en-US/firefox/50.0/releasenotes/
[2] https://en.wikipedia.org/wiki/Firefox_version_history#Versions_50_through_59
msg38830 (view) Author: [hidden] (ced) (Tryton committer) (Tryton translator) Date: 2018-03-06.17:05:14
When was released firefox 50? How long is a release supported?
msg38829 (view) Author: [hidden] (pokoli) (Tryton committer) (Tryton translator) Date: 2018-03-06.16:59:41
Sao contains a specify work arround for https://bugzilla.mozilla.org/show_bug.cgi?id=841920, but this issue is resolved since firefox50 [1], so I think we can remove the hack.

On most recent versions this code also raised an error on the browser console when trying to generate the canvas blob. 


[1] https://bugzilla.mozilla.org/show_bug.cgi?id=1280584
History
Date User Action Args
2018-08-06 09:34:50reviewbotsetmessages: + msg42643
2018-08-03 01:34:33reviewbotsetmessages: + msg42623
2018-08-03 01:34:32reviewbotsetreviews: 44721002 -> 44721002, 60291002
2018-08-03 01:05:34cedsetassignedto: pokoli -> ced
superseder: + Use Material icons
messages: + msg42621
2018-04-27 14:03:59pokolisetmessages: + msg40436
2018-04-27 14:00:16pokolisetreviews: 44721002,46411002 -> 44721002
2018-04-27 13:04:32reviewbotsetmessages: + msg40434
2018-04-27 12:57:11cedsetmessages: + msg40433
2018-04-27 12:36:40pokolisetreviews: 44721002 -> 44721002,46411002
messages: + msg40432
2018-04-05 02:46:23cedlinkissue5776 superseder

Showing 10 items. Show all history (warning: this could be VERY long)