New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Viewing XML data as text #1159

Closed
ajtag opened this Issue Oct 2, 2017 · 9 comments

Comments

Projects
None yet
3 participants
@ajtag
Copy link

ajtag commented Oct 2, 2017

Details for the issue

the issue is that given some xml content in a cell, the "edit database cell" view will not render the content as text, only as binary data, claiming that the content is an image and so cannot display the text.

*i think" this bug is triggered if a cell starts with the xml header followed by valid xml data.

Useful extra information

looking at https://github.com/sqlitebrowser/sqlitebrowser/blob/master/src/EditDialog.cpp
there is a check to guess the type of cell contents, using "checkDataType(data)".

that function first checks to see if an image library can read the contents of the cell.
one assumes that the SVG image reader sees valid xml and says yes I can read that.

from that point on the cell is considered an image, and will not display text.

I'm opening this issue because:

  • DB4S is crashing
  • [ x] DB4S has a bug
  • [ x] DB4S needs a feature
  • DB4S has another problem

I'm using DB4S on:

  • Windows: ( version: ___ )
  • [ x] Linux: ( distro: ___ )
  • Mac OS: ( version: ___ )
  • Other: ___

I'm using DB4S version:

  • 3.10.1
  • 3.10.0
  • [x ] 3.9.1
  • Other: ___

I have also:

@ajtag

This comment has been minimized.

Copy link
Author

ajtag commented Oct 2, 2017

as a workaround, selecting your field as
replace(field_containing_xml, "<?xml version='1.0' encoding='utf-8'?>", '')
removes the xml declaration from the content, stopping the SVG library from stating the data is readable

you may need to adjust xml declaration to match your content

@justinclift justinclift added the bug label Oct 2, 2017

@justinclift

This comment has been minimized.

Copy link
Member

justinclift commented Oct 2, 2017

Interesting. Thanks for reporting this @ajtag. It seems like another case of #1138. That one was being triggered by any text string starting with BMS, whereas this is a different initial string triggering the same thing.

Looks like I'll need to file another bug with Qt. 😄

@justinclift justinclift referenced this issue Oct 2, 2017

Closed

To Do list for 2nd October - 6th October #109

6 of 7 tasks complete
@justinclift

This comment has been minimized.

Copy link
Member

justinclift commented Oct 3, 2017

@ajtag Filed a bug report for this with Qt: https://bugreports.qt.io/browse/QTBUG-63568

Lets give that a few days and see if it gets picked up there. If it does, we'll take a different approach for solving this in DB4S than if we need to update the DB4S code itself. 😄

MKleusberg added a commit that referenced this issue Oct 5, 2017

Use a more sophisticated method for detecting image data in Edit dialog
In the Edit Data dialog we check if the data contained in a cell is an
image. This check returned some false positives, so this commit adds
another more sophisticated check to work around that.

See issue #1138 and #1159.
@MKleusberg

This comment has been minimized.

Copy link
Member

MKleusberg commented Oct 5, 2017

I'm not entirely sure if this is actually a Qt bug because in their documentation they write:

canRead() is a lightweight function that only does a quick test to see if the image data is valid. read() may still return false after canRead() returns true, if the image data is corrupt.

(Source: https://doc.qt.io/qt-5/qimagereader.html#canRead)
So it's meant to be that way for performance reasons I guess.

And it's easy to fix this for us, too 😄 I've just added the required code, so it'll be in the next nightly build. Can you download and test the next nightly build, @ajtag, and check if it's working for you as expected now? 😄

@MKleusberg

This comment has been minimized.

Copy link
Member

MKleusberg commented Oct 5, 2017

Just for clarification: I think the Qt bug report by @justinclift is still valid. We can easily work around this issue in our code but we still profit from it if the Qt developers change the canRead() function because it would (slightly) increase the performance of the Edit Data dialog, especially for large files.

@ajtag

This comment has been minimized.

Copy link
Author

ajtag commented Oct 5, 2017

currently the viewer does not render the image now when it recieves the null from the render later on.
it also looks like the fix passes the "well it fixed my bug" test.
like you say, it is a shame to have to parse the image to set the display type. but it is going to work.

Thanks for picking this up.

Angus

@justinclift

This comment has been minimized.

Copy link
Member

justinclift commented Oct 6, 2017

currently the viewer does not render the image now when it recieves the null from the render later on.

Reading that line, I'm not sure what you're meaning. Might be just due to me being really tired though. 😄

@ajtag

This comment has been minimized.

Copy link
Author

ajtag commented Oct 6, 2017

sorry, thats me not reading what I was typing..
the old behaviour tried to show an image and failed,
image

I have just pulled the latest version and built it, with success \o/
it now defaults to the text type
now when selecting the image view it, as below it presents the correct information
image

Thats great.
Thanks for your help.

@ajtag ajtag closed this Oct 6, 2017

@justinclift

This comment has been minimized.

Copy link
Member

justinclift commented Oct 6, 2017

Awesome, thanks for verifying @ajtag.

@MKleusberg Looks like the second level check approach works well. Well done. 😄

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment