My 995$e is mapped to the a AV and I got the following dropdown list:

<option value="Langues étrangères"></option>
<option value="Ados">Ados</option>
<option value="Album">Album</option>
<option value="BD adulte">BD adulte</option>
<option value="BD enfant">BD enfant</option>
<option value="Bureau">Bureau</option>
<option value="Cédéroms adulte"></option>
<option value="Cédéroms enfant"></option>
<option value="Contes">Contes</option>

etc.

When the description contains a diacritic, the option is empty.

Marked as Failed QA.

(In reply to Jonathan Druart from comment #4)
> 
> When the description contains a diacritic, the option is empty.
> 
> Marked as Failed QA.

Right, it's true, encoding problems.
But this is one of the problems fixed by Bug 11944. 
Tested on top of that and it works :)

Can you test it that way?
I'm adding 11944 as a dependency.

Bug 11944 fixes the issue I raised on comment 4.

Patch pushed to master.

Thanks Bernardo!

(In reply to Tomás Cohen Arazi from comment #8)
> Patch pushed to master.

Tomás, without bug 11944, this patch introduces a regression (see comment 4), I am not sure it should have been pushed before it.

Jonathan, thanks for pointing this out. I have reverted this patch, and will push it when the UTF-8 work hits master.

Adding a see also on this one to bug 10480 (work in progress). It will touch this part of code too in particular to item plugins. Not a hard rebase on either side..

Bernardo, could you please rebase this one? I want it :-D

Make sure that bug 7673 is not regressed.

Ping

Looking at this one now..

Thanks Bernardo for reviving this one.
The patch was fine already although I eventually added a list of seven points..
Hopefully the QAer of duty will not be too hard on us :)

(In reply to Marcel de Rooy from comment #20)
> Thanks Bernardo for reviving this one.

Thank you!!

> 
> [1] Remove .hidden after [% avalue %] in additem.tt. (Typo)
a leftover from my debugging 

> [2] A closer look revealed that mv.avalue is useless too. An input element
>     does not contain any content. Putting it after the hidden element
>     in a non-visible context has no meaning.
Ok!
> [3] Change handling of select attributes readonly and disabled.

I was worried about this, since i think select can't have readonly
attribute, only disabled. But if disabled, it's content is not sent
back to GET/POST, so perhaps a hidden attribute with same name and
old value is needed.
Or may be now select can have readonly, but then we need to remove disabled.
I left both to not change old behavior

> [4] Remove unused variable $attributes_no_value_textarea
Good! I forgot them
But in my first patch i also removed "my $attributes_no_value"

> 
> Sorry that this small list kept growing :)
Perfect, thanks again!

(In reply to Bernardo Gonzalez Kriegel from comment #22)
> > [3] Change handling of select attributes readonly and disabled.
> 
> I was worried about this, since i think select can't have readonly
> attribute, only disabled. But if disabled, it's content is not sent
> back to GET/POST, so perhaps a hidden attribute with same name and
> old value is needed.
> Or may be now select can have readonly, but then we need to remove disabled.
> I left both to not change old behavior

Seems good to me. I recall seeing some code in the template that removes the disabled status when submitting the form ..

On editing an item, I get

Form not submitted because of the following problem(s)
------------------------------------------------------------------------------------

- 4 mandatory fields empty (highlighted)

Which is wrong, my 4 mandatory fields are filled.

(In reply to Jonathan Druart from comment #24)
> On editing an item, I get
> 
> Form not submitted because of the following problem(s)
> -----------------------------------------------------------------------------
> -------
> 
> - 4 mandatory fields empty (highlighted)
> 
> Which is wrong, my 4 mandatory fields are filled.

Good catch. I see that something else is wrong not related to mandatory fields but indirectly causing this issue..

Bernardo:
Could you please retest with the latest patch attached and verify that we now resolved these issues (and add signoff)?

Could you also confirm that we do no longer need select parameters:
-                    -labels   => \%authorised_lib,
-                    -override => 1,
-                    -multiple => 0,
These were passed to CGI::scrolling_list in the past.

Thanks.

(In reply to Marcel de Rooy from comment #27)
> Could you please retest with the latest patch attached and verify that we
> now resolved these issues (and add signoff)?

Done :)

> 
> Could you also confirm that we do no longer need select parameters:
> -                    -labels   => \%authorised_lib,
This IS passed ( labels   => \%authorised_lib, )
and we need it.

> -                    -override => 1,
It's something from CGI, no needed

> -                    -multiple => 0,
multiple='false' is default, so no needed

> Thanks.

Thanks to you!

Sorry but I have found something else.
Before this patch, the item line which is currently edited was highlighted in yellow. After this patch, it does not anymore.

(In reply to Jonathan Druart from comment #32)
> Sorry but I have found something else.
> Before this patch, the item line which is currently edited was highlighted
> in yellow. After this patch, it does not anymore.

Jonathan: I also remember the yellow line. But when I am testing in current clean master now, I do not see it anymore !
I am looking where it is triggered in the template/js ?

(In reply to Marcel de Rooy from comment #33)
> (In reply to Jonathan Druart from comment #32)
> > Sorry but I have found something else.
> > Before this patch, the item line which is currently edited was highlighted
> > in yellow. After this patch, it does not anymore.
> 
> Jonathan: I also remember the yellow line. But when I am testing in current
> clean master now, I do not see it anymore !
> I am looking where it is triggered in the template/js ?

Found somethings else. See bug 14267.
But it seems that we did not have a yellow background in the input field. Note that the item currently edited is still yellow in the table.
Checked on older version too.
Please provide a screen shot from current master to convince me if I am not correct.
Back to Signed off.

(In reply to Marcel de Rooy from comment #33)
> (In reply to Jonathan Druart from comment #32)
> > Sorry but I have found something else.
> > Before this patch, the item line which is currently edited was highlighted
> > in yellow. After this patch, it does not anymore.
> 
> Jonathan: I also remember the yellow line. But when I am testing in current
> clean master now, I do not see it anymore !
> I am looking where it is triggered in the template/js ?

By item line I meant the tr tag in the items table.
Screenshot coming.

And I confirm that it does not work with this patch set.

(In reply to Jonathan Druart from comment #37)
> And I confirm that it does not work with this patch set.

Hey, it works... half of the time :)
Only highlights even items, odd ones remains un-highligthed.

OK One misunderstanding resolved..
The items table highlight should not be that hard to fix.
Will have a look.

(In reply to Jonathan Druart from comment #37)
> And I confirm that it does not work with this patch set.

I have the strong impression that it "does not work" without this patch set too :) It only works on the even items.
So imo this should not interfere with this report. But let me add a small followup for changing behavior (it should go on another report).

(In reply to Marcel de Rooy from comment #41)
> Created attachment 39513 [details] [review] [review]
> Highlight

Would this resolve your highlight problem, Jonathan? No clean patch btw..

Created attachment 39517 [details] [review]
Bug 12176: Remove HTML from additem.pl

This patch removes HTML code from additem.pl.

To test:
1. Check no regressions on Add/Edit/Save items
2. Update translation files for a language,
   check new strings "Tag editor" & "No popup" on staff PO file
3. Check it passes xt/tt_valid.t

Patch partially rebased, part rewritten.

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>

Signed-off-by: Jonathan Druart <jonathan.druart@koha-community.org>

Created attachment 39518 [details] [review]
Bug 12176: [QA Follow-up] Small additem adjustments

Adjusting a few small things and making qa tools happy:

[1] Remove .hidden after [% avalue %] in additem.tt. (Typo)
[2] A closer look revealed that mv.avalue is useless too. An input element
    does not contain any content. Putting it after the hidden element
    in a non-visible context has no meaning.
[3] Change handling of select attributes readonly and disabled.
[4] Remove unused variable $attributes_no_value_textarea
[5] Removed a comment with TODO referring to this report.
[6] Moving a duplicated TT variable (mv.javascript) outside IF statement.
[7] And finally could not resist this one: Moving strings Tag editor and
    No popup from script to template.  Plugins++
    NOTE: Most item plugins redirect click to focus. In that case there is
    no popup, but unfortunately the text Tag editor comes up.
    When you remove or rename function Click, No popup comes up. So it works.

Sorry that this small list kept growing :)

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Bernardo Gonzalez Kriegel <bgkriegel@gmail.com>

Signed-off-by: Jonathan Druart <jonathan.druart@koha-community.org>

Created attachment 39519 [details] [review]
Bug 12176: [QA Follow-up] Capitalization typo after eleventh hour

While cleaning up, still found this super tiny string typo :)
Yes, Tag Editor should now be Tag editor.

Test plan :)
Git grep on Tag editor and Tag Editor

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Bernardo Gonzalez Kriegel <bgkriegel@gmail.com>

Signed-off-by: Jonathan Druart <jonathan.druart@koha-community.org>

Created attachment 39520 [details] [review]
Bug 12176: Fix for missing field_value in select

As Jonathan Druart discovered, we were still missing an important attribute
in the selects. Var mv.name was not filled, so the name was empty with nice
side-effects :)
While fixing this, it was also possible to delete some unused vars that Bernardo
already mentioned before: $attributes and $attributes_no_value.

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>

Signed-off-by: Bernardo Gonzalez Kriegel <bgkriegel@gmail.com>

For me the simptom was different, when saving an edited item
fields were mangled beyond repear :(
No alert for empty mandatory fields.

But this last patch fix the problem.
Tested add/edit items
Cleaner pl file :)
No errors

Signed-off-by: Jonathan Druart <jonathan.druart@koha-community.org>

(In reply to Marcel de Rooy from comment #40)
> (In reply to Jonathan Druart from comment #37)
> > And I confirm that it does not work with this patch set.
> 
> I have the strong impression that it "does not work" without this patch set
> too :) It only works on the even items.
> So imo this should not interfere with this report. But let me add a small
> followup for changing behavior (it should go on another report).

Yes you are right, it does not work as expected on master.
So let's move it to its own bug report.

Patches pushed to master.

Good job Bernardo and Marcel!

Pushed to 3.20.x will be in 3.20.1

Hi,

these patches will not apply cleanly on 3.18.x, please resubmit if necessary.

Cheers,
Liz