Closed (fixed)
Project:
Translation Management Tool
Version:
8.x-1.x-dev
Component:
User interface
Priority:
Major
Category:
Bug report
Assigned:
Unassigned
Reporter:
Created:
1 Sep 2015 at 10:28 UTC
Updated:
7 Oct 2015 at 16:04 UTC
Jump to comment: Most recent, Most recent file

Comments
Comment #2
juanse254 commentedAnd this should fix it.
Comment #3
juanse254 commentedHum sorry, this is the right TEST-Only.
Comment #6
juanse254 commentedComment #12
juanse254 commentedThat was not working, this is a better aproach.
Comment #17
juanse254 commentedThe test remains the same as in patch #12, as you can see in the interdiff.
Comment #20
juanse254 commentedComment #21
edurenye commentedThe code seems fine and it works fine.

I'm providing an snapshot of the result:
Comment #22
berdirThis seems more complicated than it has to be. Instead of creating two arrays and merging them, why don't you just move the if/else below and just set the additional key in there? At that point, it might be simple enough to do it as a ternary?
Comment #23
juanse254 commentedYou are right, this should work and save some lines.
Comment #24
giancarlosotelo commentedThe patch looks good but I tested locally and some links are not working.
You should also fix the link in.
and add some test for that.
Comment #25
juanse254 commentedThis might solve it, some tests needed for the changes.
Comment #26
giancarlosotelo commentedNow looks good! but we need tests.
Comment #27
giancarlosotelo commentedComment #28
edurenye commentedIf it needs tests, should go to needs work.
Comment #29
juanse254 commentedThis is the only way to test that, couldn't test the 'being processed' because it doesn't seem to be used anywhere.
Comment #30
juanse254 commentedThis will fix both breaking tests and this issue, had to put them together since they are strictly related as the fail in the same message. Im also promoting to major since tests area failing until this gets fixed.
Comment #31
juanse254 commentedComment #34
juanse254 commentedOkay a couple minor changes due to #2565895: Add a new :placeholder to SafeMarkup::format() for URLs that handles bad protocols. I hope this works.
Comment #35
edurenye commentedThere is a tabulation that should not be there, but this issue fixes the head, and as it passes the tests and the code seems fine.
I set it to RTBC, as should be commited asap and maybe delete the tabulation on commiting.
Comment #36
juanse254 commentedIm sorry that patch contained an unrelated interdiff. This is the one.
Comment #37
juanse254 commentedOmg, im sorry a new patch got committed meanwhile and i forgot to rebase.
Comment #38
giancarlosotelo commentedOk it seems that the head is fixed.
Comment #39
berdirGreat, committed.