This revision was automatically updated to reflect the committed changes.
Closed by commit R223:4b2ba6a7af25: Try to display a malformed image if
feasible (authored by trufanov).
REPOSITORY
R223 Okular
CHANGES SINCE LAST UPDATE
https://phabricator.kde.org/D8415?vs=22327=22331
REVISION
trufanov updated this revision to Diff 22327.
trufanov added a comment.
Wording of a warning message is changed.
With dot at sentence end.
A screenshot of warning appearance is
attached{https://phabricator.kde.org/F5491726}
REPOSITORY
R223 Okular
CHANGES SINCE LAST UPDATE
ngraham added a comment.
Actually @trufanov, would you mind doing it? I'd rather not commandeer your
revision or commit it by hand, both of which will break the history here on
phabricator.
REPOSITORY
R223 Okular
REVISION DETAIL
https://phabricator.kde.org/D8415
To: trufanov, ngraham,
ngraham added a comment.
OK, will do!
REPOSITORY
R223 Okular
REVISION DETAIL
https://phabricator.kde.org/D8415
To: trufanov, ngraham, #okular, aacid
Cc: aacid, progwolff, ngraham
trufanov added a comment.
@ngraham Feel free to do it yourself.
REPOSITORY
R223 Okular
REVISION DETAIL
https://phabricator.kde.org/D8415
To: trufanov, ngraham, #okular, aacid
Cc: aacid, progwolff, ngraham
ngraham accepted this revision.
ngraham added a comment.
@trufanov, would you like to make that change? If not I can do it, and either
way, let's land this tonight.
REPOSITORY
R223 Okular
REVISION DETAIL
https://phabricator.kde.org/D8415
To: trufanov, ngraham, #okular, aacid
Cc: aacid,
aacid accepted this revision.
aacid added a comment.
Personally i'd change the : for a .
But i don't really care, go ahead i'd say.
REPOSITORY
R223 Okular
REVISION DETAIL
https://phabricator.kde.org/D8415
To: trufanov, ngraham, #okular, aacid
Cc: aacid, progwolff, ngraham
ngraham edited the summary of this revision.
REPOSITORY
R223 Okular
REVISION DETAIL
https://phabricator.kde.org/D8415
To: trufanov, ngraham, #okular, aacid
Cc: aacid, progwolff, ngraham
ngraham added a comment.
Yeah, in the above screenshot, `reader.errorString()` seems to have returned
"Unable to read image data" which is not very helpful. How about a single line
and the following text:
> This document appears malformed. Here is a best approximation of the
document's
aacid added a comment.
Sounds good, do we actually get a meaningful error message in this case?
If not i'd say just remove the "Error: [error message]" part.
REPOSITORY
R223 Okular
REVISION DETAIL
https://phabricator.kde.org/D8415
To: trufanov, ngraham, #okular, aacid
Cc: aacid,
ngraham added a comment.
I am, yes. How does this sound?
> This document appears malformed. Error: [error message]
> Here is a best approximation of the document's intended appearance:
This wording (or something similar) has the benefit that it casts Okular as a
hero--bravely
aacid added a comment.
@ngraham you seem like a native speaker, please commit this with proper
english-ness
REPOSITORY
R223 Okular
REVISION DETAIL
https://phabricator.kde.org/D8415
To: trufanov, ngraham, #okular, aacid
Cc: aacid, progwolff, ngraham
trufanov added a comment.
Ok, I've added a warning for this. Please check wording - english isn't my
native language. Feel free to change it.
Attached are screenshots of Okular showing current warning for eng and rus
locales.F5489936: Screenshot_20171113_113818.png
ngraham added a comment.
That's a good idea. @trufanov, can you add that?
REPOSITORY
R223 Okular
REVISION DETAIL
https://phabricator.kde.org/D8415
To: trufanov, ngraham, #okular, aacid
Cc: aacid, progwolff, ngraham
aacid accepted this revision.
aacid added a comment.
This revision is now accepted and ready to land.
Sure, commit it, but when you do can you please emit a warning() message
saying something like "the file has some errors we're showing the best we can"?
REPOSITORY
R223 Okular
REVISION
ngraham added a comment.
Could we add the local workaround here, and include a FIXME comment urging an
upstream fix later? Gwenview already has this workaround...
REPOSITORY
R223 Okular
REVISION DETAIL
https://phabricator.kde.org/D8415
To: trufanov, ngraham, #okular
Cc: aacid,
trufanov added a comment.
In https://phabricator.kde.org/D8415#162014, @aacid wrote:
> In https://phabricator.kde.org/D8415#160837, @trufanov wrote:
>
> > In https://phabricator.kde.org/D8415#159049, @aacid wrote:
> >
> > > Please consider fixing this in Qt before asking us to
trufanov added a comment.
In https://phabricator.kde.org/D8415#159049, @aacid wrote:
> Please consider fixing this in Qt before asking us to create a workaround.
I'm fine with workarounded gwenview as well as working GIMP and ImageMagick.
REPOSITORY
R223 Okular
REVISION DETAIL
aacid added a comment.
Please consider fixing this in Qt before asking us to create a workaround.
REPOSITORY
R223 Okular
REVISION DETAIL
https://phabricator.kde.org/D8415
To: trufanov, ngraham, #okular
Cc: aacid, progwolff
progwolff added a comment.
In https://phabricator.kde.org/D8415#158804, @aacid wrote:
> Wouldn't it make more sense to fix this in QImageReader and not in every
user of QImageReader ?
From my perspective the behaviour of QImageReader is correct.
QImage::read docs:
>
aacid added a comment.
Wouldn't it make more sense to fix this in QImageReader and not in every user
of QImageReader ?
REPOSITORY
R223 Okular
REVISION DETAIL
https://phabricator.kde.org/D8415
To: trufanov, ngraham, #okular
Cc: aacid, progwolff
progwolff added a comment.
In https://phabricator.kde.org/D8415#158800, @trufanov wrote:
> Also I heard I can add
> Differential Revision: https://phabricator.kde.org/D8415
> in commit message to automatically close phabricator review. Is it so?
This is true, but it must be
trufanov added a comment.
Ok, I've edited summary. Didn't know that KDE Bugs could be referred in a
such way in phabricator summary instead of commit message.
Also I heard I can add
Differential Revision: https://phabricator.kde.org/D8415
in commit message to automatically close
trufanov edited the summary of this revision.
REPOSITORY
R223 Okular
REVISION DETAIL
https://phabricator.kde.org/D8415
To: trufanov, ngraham, #okular
Cc: aacid
ngraham added a comment.
Thanks for the patch: The Summary needs some reformatting:
- Add the special keyword "BUG 385384" on its own line
- Remove the quote from the Bugzilla ticket; instead briefly explain the
change in your own words
- No need to explain the code change; it should
trufanov created this revision.
trufanov added reviewers: ngraham, Okular.
trufanov added a project: Okular.
REVISION SUMMARY
resolves bug https://bugs.kde.org/show_bug.cgi?id=385384
Bug 385384 - Some PNG files produced by acdsee can't be displayed
> Hi,
> I've got a png file that
26 matches
Mail list logo
