Erreur de Javascript #4863

Open
opened 3 months ago by manu0111 · 27 comments

Ceci s'observe partout où il y a un formulaire HTML de versement de document. Par exemple dans l'encart « Ajouter une image ou un document » de l'édition des articles /ecrire/?exec=article_edit&new=oui

Le formulaire HTML classique disparait une fraction de seconde après le chargement de lapage, et le formulaire bigup n'apparait pas.

La console JS indique
Uncaught Invalid object

Ca se passe ici sur la ligne this.flow.assignBrowse au cactère 11.

definir_zone_depot:function(){
//Cacher l'input original
this.input.hide();
this.creer_zone_depot();
//Voir la zone,si on n'a pas déjÃ| atteint le quota de fichiers
this.adapter_visibilite_zone_depot();
//Assigner la zone et son boutonÃ| flow.
this.flow.assignBrowse(
this.zones.depot.find('.dropfilebutton'),
false,
!this.multiple,
{accept:this.opts.contraintes.accept}
);
this.assignDropExtended(this.zones.depot_etendu);
},
Ceci s'observe partout où il y a un formulaire HTML de versement de document. Par exemple dans l'encart « Ajouter une image ou un document » de l'édition des articles /ecrire/?exec=article_edit&new=oui Le formulaire HTML classique disparait une fraction de seconde après le chargement de lapage, et le formulaire bigup n'apparait pas. La console JS indique Uncaught Invalid object Ca se passe ici sur la ligne this.flow.assignBrowse au cactère 11. ```js definir_zone_depot:function(){ //Cacher l'input original this.input.hide(); this.creer_zone_depot(); //Voir la zone,si on n'a pas déjÃ| atteint le quota de fichiers this.adapter_visibilite_zone_depot(); //Assigner la zone et son boutonÃ| flow. this.flow.assignBrowse( this.zones.depot.find('.dropfilebutton'), false, !this.multiple, {accept:this.opts.contraintes.accept} ); this.assignDropExtended(this.zones.depot_etendu); }, ```

il faudrait que tu précise le navigateur, sa version, et l'OS

il faudrait que tu précise le navigateur, sa version, et l'OS
Owner

Je me suis permis d’éditer pour avoir la coloration syntaxique.

Je me suis permis d’éditer pour avoir la coloration syntaxique.
b_b added the
bug
label 3 months ago
Owner

Version utilisée ?

Version utilisée ?
Poster

J'obtiens ce bug avec SPIP 4.1.5 GIT [4.1: 60362615] et l'écran de sécurité 1.4.2

Coté client, je le reproduis avec
Opera/Mac 90.0.4480.48
Safari/Mac 13.1.2 (13609.3.5.1.5)
Chrome/Mac 104.0.5112.101
Firefox/Mac 103.0.2
Firefox/NetBSD 100.0.2 (ça ne vient donc pas du mac...)

J'ai une backtrace:

Uncaught TypeError: this.flow.assignBrowse is not a function
definir_zone_depot
    gerer_depot_fichiers
    inputs_a_gerer
    each
    each
    bigup
    formulaires_avec_bigup
    setTimeout handler*formulaires_avec_bigup
    <anonymous>
    mightThrow
    process
    setTimeout handler*Deferred/then/resolve/<
    fire
    fireWith
    fire
J'obtiens ce bug avec SPIP 4.1.5 GIT [4.1: 60362615] et l'écran de sécurité 1.4.2 Coté client, je le reproduis avec Opera/Mac 90.0.4480.48 Safari/Mac 13.1.2 (13609.3.5.1.5) Chrome/Mac 104.0.5112.101 Firefox/Mac 103.0.2 Firefox/NetBSD 100.0.2 (ça ne vient donc pas du mac...) J'ai une backtrace: ```js Uncaught TypeError: this.flow.assignBrowse is not a function definir_zone_depot gerer_depot_fichiers inputs_a_gerer each each bigup formulaires_avec_bigup setTimeout handler*formulaires_avec_bigup <anonymous> mightThrow process setTimeout handler*Deferred/then/resolve/< fire fireWith fire ```
Owner

Ça me parait étonnant que this.flow existe, mais pas this.flow.assignBrowse() tout de même.

Tu as des plugins qui pourraient intéragir avec cela ou rien d’autre ?
C’est un SPIP neuf ? ou tu as un historique dessus ?

Ça me parait étonnant que `this.flow` existe, mais pas `this.flow.assignBrowse()` tout de même. Tu as des plugins qui pourraient intéragir avec cela ou rien d’autre ? C’est un SPIP neuf ? ou tu as un historique dessus ?
Poster

J'ai tenté en désactivant tous les plugins, même peine.

Ca me le fait sur tous les sites après passage de 3.2 en 4.1. Ils ont un historique plus ou moins long.

J'ai tenté en désactivant tous les plugins, même peine. Ca me le fait sur tous les sites après passage de 3.2 en 4.1. Ils ont un historique plus ou moins long.
Owner

Est-ce qu’il se pourrait que tu aies un fichier ~/lib/flow/flow.js ou ~/squelettes/lib/flow/flow.js ou qqc comme ça qui prenne le pas sur la librairie fournie par Bigup ?


Tes fichiers (spip) sont-ils gérés en Git ou les envoies tu en FTP ou SCP ou spip_loader ou je ne sais quoi ?

Et / ou est-ce que tu peux passer à l’occasion sur l’IRC et donner un accès (admin) sur l’un des sites à l’un de nous pour regarder ?

Est-ce qu’il se pourrait que tu aies un fichier `~/lib/flow/flow.js` ou `~/squelettes/lib/flow/flow.js` ou qqc comme ça qui prenne le pas sur la librairie fournie par Bigup ? ---- Tes fichiers (spip) sont-ils gérés en Git ou les envoies tu en FTP ou SCP ou spip_loader ou je ne sais quoi ? Et / ou est-ce que tu peux passer à l’occasion sur l’IRC et donner un accès (admin) sur l’un des sites à l’un de nous pour regarder ?
Poster

Le seul fichier flow.js sur le système est spip/plugins-dist/bigup/lib/flow/flow.js

L'installation est gérée en git. C'est compliqué pour donner un accès.

Sinon, j'ai un joli patch pour rendre bigup configurable dans l'espace privé, comme il l'est actuellement dans l'espace public. Je trouve qu'il y a une certaine vertu à rendre débrayable les nouvelles fonctionnalités qui peuvent casser. Surtout quand elles reposent sur des millieurs de ligne de JS.

Le seul fichier flow.js sur le système est spip/plugins-dist/bigup/lib/flow/flow.js L'installation est gérée en git. C'est compliqué pour donner un accès. Sinon, j'ai un joli patch pour rendre bigup configurable dans l'espace privé, comme il l'est actuellement dans l'espace public. Je trouve qu'il y a une certaine vertu à rendre débrayable les nouvelles fonctionnalités qui peuvent casser. Surtout quand elles reposent sur des millieurs de ligne de JS.
b_b added this to the spip-4.1 milestone 3 months ago
Owner

Je te dirais bien qu’il te «suffit» de supprimer plugins-dist/bigup dans ce cas :)

Mais ce n’est pas tant l’option de désactivation qui nous intéresse que de comprendre le problème véritablement… parce que a priori tu es le premier à nous signaler celui là précisemment à ma connaissance.

Le Uncaught Invalid object pourrait laisser supposer que le compresseur a raté quelque chose dans la minification du fichier flow.js peut être ?

Est-ce que ajouter (dans config/mes_options.php temporairement) ce code change quelque chose dans l’erreur ?

define('_INTERDIRE_COMPACTE_HEAD_ECRIRE', true);
Je te dirais bien qu’il te «suffit» de supprimer plugins-dist/bigup dans ce cas :) Mais ce n’est pas tant l’option de désactivation qui nous intéresse que de comprendre le problème véritablement… parce que a priori tu es le premier à nous signaler celui là précisemment à ma connaissance. Le `Uncaught Invalid object` pourrait laisser supposer que le compresseur a raté quelque chose dans la minification du fichier `flow.js` peut être ? Est-ce que ajouter (dans config/mes_options.php temporairement) ce code change quelque chose dans l’erreur ? ```php define('_INTERDIRE_COMPACTE_HEAD_ECRIRE', true); ```
Poster

Oui, avec _INTERDIRE_COMPACTE_HEAD_ECRIRE l'erreur disparait, et bigup fonctionne.

Oui, avec _INTERDIRE_COMPACTE_HEAD_ECRIRE l'erreur disparait, et bigup fonctionne.
Owner

Ha, ça serait un problème similaire à spip/compresseur#4838 cf spip/compresseur#4838 ?

Ha, ça serait un problème similaire à https://git.spip.net/spip/compresseur/issues/4838 cf https://git.spip.net/spip/compresseur/issues/4838#issuecomment-29279 ?
Poster

Ca a tout à fait l'air d'être une réincarnation du bug spip/compresseur#4838

J'ai enlevé du mes_options.php
define('_INTERDIRE_COMPACTE_HEAD_ECRIRE', true);

A la place, j'ai mis:
ini_set("pcre.jit", 0);

Ca résout le bug, qui est pourtant censé être corrigé en 4.1, non?

Ca a tout à fait l'air d'être une réincarnation du bug spip/compresseur#4838 J'ai enlevé du mes_options.php define('_INTERDIRE_COMPACTE_HEAD_ECRIRE', true); A la place, j'ai mis: ini_set("pcre.jit", 0); Ca résout le bug, qui est pourtant censé être corrigé en 4.1, non?
Owner

Censé être corrigé c'est un grand mot car

  1. c'est un bug de pcre donc on peut pas le corriger, tout au mieux essayer de le contourner - la seule bonne solution c'est de up PHP/pcre
  2. on a pas eu beaucoup de feedback de gens ayant eu le problème, donc l'échantillon de test est assez réduit.

Mais si tu peux compléter le ticket spip/compresseur#4838 en indiquant la valeur de la constante PCRE_VERSION chez toi et ce que renvoie ini_get("pcre.jit") ça ferait avancer le problème. Et si jamais tu peux même trouver la/les regpexp à commenter pour éviter de tomber sur le soucis ça serait même encore plus top !

Censé être corrigé c'est un grand mot car 1. c'est un bug de pcre donc on peut pas le corriger, tout au mieux essayer de le contourner - la seule bonne solution c'est de up PHP/pcre 2. on a pas eu beaucoup de feedback de gens ayant eu le problème, donc l'échantillon de test est assez réduit. Mais si tu peux compléter le ticket https://git.spip.net/spip/compresseur/issues/4838 en indiquant la valeur de la constante `PCRE_VERSION` chez toi et ce que renvoie `ini_get("pcre.jit")` ça ferait avancer le problème. Et si jamais tu peux même trouver la/les regpexp à commenter pour éviter de tomber sur le soucis ça serait même encore plus top !
Owner

On peut donc fermer ce ticket et continuer l'échange sur spip/compresseur#4838 ?

On peut donc fermer ce ticket et continuer l'échange sur https://git.spip.net/spip/compresseur/issues/4838 ?
Poster

Différence avec spip/compresseur#4838

Le contournement de bug est bien actif: dans ce bloc issu de /plugins-dist/compresseur/lib/JavascriptPacker/class.JavaScriptPacker.php, je sors effectivement avec PHP_ISSUE_81101 à true:

                if (defined("PCRE_VERSION")
                        and strpos(PCRE_VERSION, "10.37") === 0
                        and ini_get("pcre.jit")) {
                        $this->PHP_ISSUE_81101 = true;
                }

En revanche, la comparaison des JS générés avec pcre.jit à 0 ou 1 montre une différence inattendue, il y a un bloc de code qui disparait quand pcre.jit est à 1. Voila le chunk concerné du diff entre version (-, à gauche) avec pcre.jit=0 et version (+, à droite) avec pcre.jit=1

@@ -9230,8 +11303,9 @@
 if(found){
 return found;
 }
 }
+//Now,simply look for the next,best thing to upload
 each(this.files,function(file){
 if(!file.paused){
 each(file.chunks,function(chunk){
 if(chunk.status()==='pending'){
@@ -9247,62 +11321,25 @@
 });
 if(found){
 return true;
 }
+//The are no more outstanding chunks to upload,check is everything is done
 var outstanding=false;
 each(this.files,function(file){
 if(!file.isComplete()){
 outstanding=true;
 return false;
 }
 });
 if(!outstanding&&!preventEvents){
+//All chunks have been uploaded,complete
 async(function(){
 this.fire('complete');
 },this);
 }
 return false;
 },
-assignBrowse:function(domNodes,isDirectory,singleFile,attributes){
-if(domNodes instanceof Element){
-domNodes=[domNodes];
-}
-each(domNodes,function(domNode){
-var input;
-if(domNode.tagName==='INPUT'&&domNode.type==='file'){
-input=domNode;
-}else{
-input=document.createElement('input');
-input.setAttribute('type','file');
-extend(input.style,{
-visibility:'hidden',
-position:'absolute',
-width:'1px',
-height:'1px'
-});
-domNode.appendChild(input);
-domNode.addEventListener('click',function(){
-input.click();
-},false);
-}
-if(!this.opts.singleFile&&!singleFile){
-input.setAttribute('multiple','multiple');
-}
-if(isDirectory){
-input.setAttribute('webkitdirectory','webkitdirectory');
-}
-each(attributes,function(value,key){
-input.setAttribute(key,value);
-});
-var $=this;
-input.addEventListener('change',function(e){
-if(e.target.value){
-$.addFiles(e.target.files,e);
-e.target.value='';
-}
-},false);
-},this);
-},
+ 
 assignDrop:function(domNodes){
 if(typeof domNodes.length==='undefined'){
 domNodes=[domNodes];
 }
Différence avec spip/compresseur#4838 Le contournement de bug est bien actif: dans ce bloc issu de /plugins-dist/compresseur/lib/JavascriptPacker/class.JavaScriptPacker.php, je sors effectivement avec PHP_ISSUE_81101 à true: ``` if (defined("PCRE_VERSION") and strpos(PCRE_VERSION, "10.37") === 0 and ini_get("pcre.jit")) { $this->PHP_ISSUE_81101 = true; } ``` En revanche, la comparaison des JS générés avec pcre.jit à 0 ou 1 montre une différence inattendue, il y a un bloc de code qui disparait quand pcre.jit est à 1. Voila le chunk concerné du diff entre version (-, à gauche) avec pcre.jit=0 et version (+, à droite) avec pcre.jit=1 ```diff @@ -9230,8 +11303,9 @@ if(found){ return found; } } +//Now,simply look for the next,best thing to upload each(this.files,function(file){ if(!file.paused){ each(file.chunks,function(chunk){ if(chunk.status()==='pending'){ @@ -9247,62 +11321,25 @@ }); if(found){ return true; } +//The are no more outstanding chunks to upload,check is everything is done var outstanding=false; each(this.files,function(file){ if(!file.isComplete()){ outstanding=true; return false; } }); if(!outstanding&&!preventEvents){ +//All chunks have been uploaded,complete async(function(){ this.fire('complete'); },this); } return false; }, -assignBrowse:function(domNodes,isDirectory,singleFile,attributes){ -if(domNodes instanceof Element){ -domNodes=[domNodes]; -} -each(domNodes,function(domNode){ -var input; -if(domNode.tagName==='INPUT'&&domNode.type==='file'){ -input=domNode; -}else{ -input=document.createElement('input'); -input.setAttribute('type','file'); -extend(input.style,{ -visibility:'hidden', -position:'absolute', -width:'1px', -height:'1px' -}); -domNode.appendChild(input); -domNode.addEventListener('click',function(){ -input.click(); -},false); -} -if(!this.opts.singleFile&&!singleFile){ -input.setAttribute('multiple','multiple'); -} -if(isDirectory){ -input.setAttribute('webkitdirectory','webkitdirectory'); -} -each(attributes,function(value,key){ -input.setAttribute(key,value); -}); -var $=this; -input.addEventListener('change',function(e){ -if(e.target.value){ -$.addFiles(e.target.files,e); -e.target.value=''; -} -},false); -},this); -}, + assignDrop:function(domNodes){ if(typeof domNodes.length==='undefined'){ domNodes=[domNodes]; } ```
Owner

C'est la suppression de ce bloc de commentaire
https://git.spip.net/spip/bigup/src/branch/master/lib/flow/flow.js#L369
qui est anormalement gourmande et supprime la fonction qui suit jusqu'à la fermeture de commentaire suivante

Cela semble donc être lié à
https://git.spip.net/spip/compresseur/src/branch/master/lib/JavascriptPacker/class.JavaScriptPacker.php#L148

Est-ce que tu peux essayer d'ajouter un U à la fin pour avoir une expression ungreedy et voir si par hasard ça corrigerait ?

- $parser->add('/\\/\\*[^*]*\\*+([^\\/][^*]*\\*+)*\\//', ' ');
+ $parser->add('/\\/\\*[^*]*\\*+([^\\/][^*]*\\*+)*\\//U', ' ');
C'est la suppression de ce bloc de commentaire https://git.spip.net/spip/bigup/src/branch/master/lib/flow/flow.js#L369 qui est anormalement gourmande et supprime la fonction qui suit jusqu'à la fermeture de commentaire suivante Cela semble donc être lié à https://git.spip.net/spip/compresseur/src/branch/master/lib/JavascriptPacker/class.JavaScriptPacker.php#L148 Est-ce que tu peux essayer d'ajouter un U à la fin pour avoir une expression ungreedy et voir si par hasard ça corrigerait ? ```diff - $parser->add('/\\/\\*[^*]*\\*+([^\\/][^*]*\\*+)*\\//', ' '); + $parser->add('/\\/\\*[^*]*\\*+([^\\/][^*]*\\*+)*\\//U', ' '); ```
Owner

(j'attends la confirmation de @manu0111 mais franchement ça me parait beaucoup plus safe avec un U)

(j'attends la confirmation de @manu0111 mais franchement ça me parait beaucoup plus safe avec un `U`)
Owner

ping @manu0111 ?

ping @manu0111 ?
Owner

Ah en fait en relisant je tenterai même plutôt

$parser->add('/\\/\\*[^*]*\\*+([^\\/*][^*]*\\*+)*?\\//', ' ');

Car en fait la regexp actuel peut en effet dépasser la fin des commentaires si \\*+ gobe tous les * sauf le dernier alors ensuite [^\\/] est vérifiée car c'est encore un * puis [^*]* est encore vérifier puisque c'est le / de fin de commentaire puis le code derrière que l'on va donc manger jusqu'au bloc de commentaire suivant si on a la chance de pas avoir de * dedans

Ah en fait en relisant je tenterai même plutôt ```php $parser->add('/\\/\\*[^*]*\\*+([^\\/*][^*]*\\*+)*?\\//', ' '); ``` Car en fait la regexp actuel peut en effet dépasser la fin des commentaires si `\\*+` gobe tous les `*` sauf le dernier alors ensuite `[^\\/]` est vérifiée car c'est encore un `*` puis `[^*]*` est encore vérifier puisque c'est le `/` de fin de commentaire puis le code derrière que l'on va donc manger jusqu'au bloc de commentaire suivant si on a la chance de pas avoir de `*` dedans
Poster
> - $parser->add('/\\/\\*[^*]*\\*+([^\\/][^*]*\\*+)*\\//', ' ');
> + $parser->add('/\\/\\*[^*]*\\*+([^\\/][^*]*\\*+)*\\//U', ' ');

Ca corrige le problème.

``` > - $parser->add('/\\/\\*[^*]*\\*+([^\\/][^*]*\\*+)*\\//', ' '); > + $parser->add('/\\/\\*[^*]*\\*+([^\\/][^*]*\\*+)*\\//U', ' '); ``` Ca corrige le problème.
Poster
> $parser->add('/\\/\\*[^*]*\\*+([^\\/*][^*]*\\*+)*?\\//', ' ');

Cette modification là ne corrige pas le bug.

``` > $parser->add('/\\/\\*[^*]*\\*+([^\\/*][^*]*\\*+)*?\\//', ' '); ``` Cette modification là ne corrige pas le bug.
Owner

ah mince j'ai fait une PR sur la deuxième. Tu peux ajouter un U sur la deuxième alors ?

$parser->add('/\\/\\*[^*]*\\*+([^\\/*][^*]*\\*+)*?\\//U', ' ');

ah mince j'ai fait une PR sur la deuxième. Tu peux ajouter un U sur la deuxième alors ? ```php $parser->add('/\\/\\*[^*]*\\*+([^\\/*][^*]*\\*+)*?\\//U', ' '); ```
Poster

ah mince j'ai fait une PR sur la deuxième. Tu peux ajouter un U sur la deuxième alors ?

Ta modification avec le U en plus, c'est bon, ça corrige le bug.

Enfin, en attendant le prochain problème. On essaye de faire de l'analyse syntaxique avec les expressions régulières, qui sont les outils de l'analyse lexicale. Je rends hommage au travail accompli pour que ça marche, mais ça reste une approche fragile. Je continue de plaider pour que ça soit débraybale dans l'espace privé.

> ah mince j'ai fait une PR sur la deuxième. Tu peux ajouter un U sur la deuxième alors ? Ta modification avec le U en plus, c'est bon, ça corrige le bug. Enfin, en attendant le prochain problème. On essaye de faire de l'analyse syntaxique avec les expressions régulières, qui sont les outils de l'analyse lexicale. Je rends hommage au travail accompli pour que ça marche, mais ça reste une approche fragile. Je continue de plaider pour que ça soit débraybale dans l'espace privé.
Owner

Je continue de plaider pour que ça soit débraybale dans l'espace privé.

La compression des scripts est bien débrayable dans le privé cf https://www.spip.net/fr_article4453.html#_INTERDIRE_COMPACTE_HEAD_ECRIRE

> Je continue de plaider pour que ça soit débraybale dans l'espace privé. La compression des scripts est bien débrayable dans le privé cf https://www.spip.net/fr_article4453.html#_INTERDIRE_COMPACTE_HEAD_ECRIRE
Poster

J'ai un effet de bord qui pointe son nez sur un autre site:

PHP Warning: preg_replace_callback(): Unknown modifier ')' in /htdocs2/export/spip-41-stabl e/plugins-dist/compresseur/lib/JavascriptPacker/class.JavaScriptPacker.php on line 626

J'ai ajouté un printf de $regexpn le problème vient d'un slash non échappé à la fin de la troisème parenthèse.

/('[^'\n\r]*')|("[^"\n\r]*")|(\/\*[^*]*\*+([^\/*][^*]*\*+)*\//)|(\s+(\/[^\/\n\r\*][^\/\n\r]*\/g?i?))|([^\w\x24\/'"*)\?:]\/[^\/\n\r\*][^\/\n\r]*\/g?i?)|((\b|\x24)[\t ]+(\b|\x24))|(([+\-])[\t ]+([+\-]))|([\t ]+)|(\s+)/
J'ai un effet de bord qui pointe son nez sur un autre site: PHP Warning: preg_replace_callback(): Unknown modifier ')' in /htdocs2/export/spip-41-stabl e/plugins-dist/compresseur/lib/JavascriptPacker/class.JavaScriptPacker.php on line 626 J'ai ajouté un printf de $regexpn le problème vient d'un slash non échappé à la fin de la troisème parenthèse. ``` /('[^'\n\r]*')|("[^"\n\r]*")|(\/\*[^*]*\*+([^\/*][^*]*\*+)*\//)|(\s+(\/[^\/\n\r\*][^\/\n\r]*\/g?i?))|([^\w\x24\/'"*)\?:]\/[^\/\n\r\*][^\/\n\r]*\/g?i?)|((\b|\x24)[\t ]+(\b|\x24))|(([+\-])[\t ]+([+\-]))|([\t ]+)|(\s+)/ ```
Poster

Pas de nouvelles sur ce ticket? On semble pourant près du but...

Pas de nouvelles sur ce ticket? On semble pourant près du but...
Owner

ah pardon @manu0111 j'avais raté ton dernier commentaire et je viens de comprendre, on ne peut donc pas ajouter de modificateur dans la regexp.

Je viens de force-pusher une version sans modificateur
bf069008c5

est-ce que tu peux me confirmer que c'est bon comme ça ?
(ie : ton js n'est pas cassé, et les commentaires /*...*/ sont bien supprimés ?)

ah pardon @manu0111 j'avais raté ton dernier commentaire et je viens de comprendre, on ne peut donc pas ajouter de modificateur dans la regexp. Je viens de force-pusher une version sans modificateur https://git.spip.net/spip/compresseur/commit/bf069008c55320abc73913c7c3a7abb28abe7b64 est-ce que tu peux me confirmer que c'est bon comme ça ? (ie : ton js n'est pas cassé, et les commentaires `/*...*/` sont bien supprimés ?)
Sign in to join this conversation.
No Milestone
No Assignees
5 Participants
Notifications
Due Date

No due date set.

Dependencies

This issue currently doesn't have any dependencies.

Loading…
There is no content yet.