Fixed in development branch svn://svn.zabbix.com/branches/dev/ZBX-5704

(2) default.css there should be good reason to make changes. So padding change and color hex change is redundant.

Eduards I hope that you understood reason of adding paddings..

tomtom No, I don't know any guidelines that "padding: 0 0 0 5px;" is superior to "padding-left: 5px;" and that "background-color: #ffffff" is superior to "background-color:#FFFFFF;"

Eduards RESOLVED

tomtom CLOSED

(3) adm.images.php $confCombobox should be $confComboBox

Eduards RESOLVED, r31074

tomtom CLOSED

(4)
Here brackets were added:
public function isValid()

Here brackets were removed:
if (!is_null($options['editable']) && self::$userData['type'] < USER_TYPE_ZABBIX_ADMIN)

Eduards Yes, if it is ternary operation we use () but in common if we use () only where it needed, for example:
if (($a > b && $b < $c) || ($a > c && $c < $b)) {}

tomtom I don't see here any ternary operation.

Eduards Any way, I don't how more precisely call this but if we have construction: return $a == $b we use () !

tomtom point me to guidelines where it is said that in "return" we use brackets and in "if statement" with to comparisions we don't!

Eduards Documented http://zabbix.org/wiki/Docs/specs/coding_style#Return_boolean_expression

tomtom CLOSED

(5) CImage.php according to sample in http://www.zabbix.org/wiki/Docs/specs/coding_style we don't put line after 'break' in 'switch' statement.

Eduards We need to discuss it, because in large switch cases it's very useful to split with empty line..

tomtom CLOSED.

(7) CImage.php line 433 why change from if "if (isset($image['image'])) {" to "(!empty($image['image'])) {"?

Eduards Because we must check if image content is empty or not..

tomtom Name example where isset() would fail. These kind of changes are very prone to errors.

Eduards Not agree at all!

tomtom this is not a case where isset() fail!

Eduards RESOLVED

tomtom CLOSED

(8) improve comments for CImage.php checkImage() method: describe all exceptions in method comment, parameter $image, magic number "10", why from $ZBX_MESSAGES are popped value.

Eduards RESOLVED, hope after this commit your question about ZBX_MESSAGE disappears. r31076

tomtom still missing comments, except magic number which were removed!

Eduards RESOLVED

tomtom CLOSED

(9) CUploadFile.php validateSize() method should be commented according to PHPDoc standart

validateSize() should be renamed to validateImageSize() as it should not be used in xml imports

Eduards RESOLVED

tomtom still comment incomplete.

Eduards RESOLVED

tomtom CLOSED

(10) in what scenario
self::exception(ZBX_API_ERROR_PARAMETERS, _s('Image "%s" has empty content.', $image['name']));
should be raised?

Eduards Why not if this is "create"??

tomtom I mean, how to get this exception in Frontend.

Eduards I have one file (.rpm) witch, I do not how, but uploads with empty content.. any way this validation extremally is necessary for API because user can not pass "image" at all or pass with empty content!

tomtom attach this file please!

Eduards RESOLVED

tomtom CLOSED

(11) I try to upload .bmp file and get error: "File format is not image. [CImage.create -> CImage.checkImage]"

Eduards Zabbix don't support bmp format, wont message "File format is not png, gif or jpg" ?

tomtom I propose to use message: "File format is unsupported".

Eduards RESOLVED, r31078

tomtom CLOSED

TESTED

Review my changes in r31117, if OK merge

Fixed in versions pre-2.1.0 (beta) r31502, pre-2.0.4rc1 r31503

(12) Please note this change in the API changelog.

Eduards RESOLVED

<richlv> ...for 2.0.5

jelisejev I've removed it for now, it will have to be added when merging to 2.0.5

Eduards Looks that bureaucracy procedure win bug fixing! Is it really open source? or we plan to became as microsoft...

<richlv> opensource doesn't equal "crap quality" and "i don't give a fuck about translators"

Eduards You think fix reverting improve quality?? I prefer not translated 1 string than not resolved bug!

(13) this was merged in a string freeze period. it added two new strings :

File format is unsupported.
Image load error [%1$s] in [%2$s].

and changed one string :

Image size must be less than 1MB -> Image size must be less than 1MB.

it must be reverted and merged again once 2.0.4 is out

Eduards REVERTED! r.31529

<richlv> CLOSED ('Image load error [%1$s] in [%2$s].' was actually added in ZBX-5737)

<richlv> actually, it was not fully reverted, incorrect changelog entry was still present in trunk changelog. removed in rev 31591.

Fixed in versions pre-2.0.5rc1 r32030