Differential D6459

Fix the National Geographic PoTD dataengine
Needs RevisionPublic
Actions

Authored by garg on Jul 1 2017, 11:26 AM.

Details

Reviewers
davidedmundson
Group Reviewers
Plasma
Summary

Ignore elements with the crossorigin attribute since these break
the DOM causing QXmlStreamReader to stop processing the XML which
leads to a empty URL.

Diff Detail

Repository
R114 Plasma Addons
Branch
master
Lint
No Linters Available
Unit
No Unit Test Coverage
garg created this revision.Jul 1 2017, 11:26 AM
Restricted Application added a project: Plasma. · View Herald TranscriptJul 1 2017, 11:26 AM
Restricted Application added a subscriber: plasma-devel. · View Herald Transcript
garg added a comment.Jul 1 2017, 11:28 AM

Unfortunately I couldn't figure out a way to relax the XML parser, so if anyone has tips on how to achieve that, I'd highly appreciate them :)

davidedmundson added a subscriber: davidedmundson.Jul 3 2017, 2:33 PM

Obviously this isn't the best fix, if they shuffle the XML about it won't work .. but if it makes things better so be it.

Do you still have a copy of this broken XML?
Is this related to the crash in XML parsing we see over bugzilla?

garg added a comment.Jul 3 2017, 2:34 PM

You can checkout the XML on http://www.nationalgeographic.com/photography/photo-of-the-day/ by just viewing the source :)

I'm not aware of a crash in bugzilla, got a bug number?

garg added a comment.Jul 4 2017, 9:22 PM

Can this also go into the next 5.10 release?

broulik added a subscriber: broulik.Jul 5 2017, 1:14 PM
broulik added inline comments.
dataengines/potd/natgeoprovider.cpp
56

Please use splitRef

(Makes me wonder why it turns it into a QString in the first place since QXmlStreamReader has an addData that takes a QByteArray

61

Use range-for, your container is const anyway

for (const auto &e : data) {
davidedmundson requested changes to this revision.Jul 10 2017, 11:36 PM

I just merged https://phabricator.kde.org/D5729 which has been accepted for ages unmerged fixing this in another way.

This revision now requires changes to proceed.Jul 10 2017, 11:36 PM

Revision Contents
Changeset List

PathPackages
Mdataengines/potd/natgeoprovider.cpp (11 lines)

Diff 16080

View Options

dataengines/potd/natgeoprovider.cpp

Loading...