Differential D3671

QtHelp page loading fix
ClosedPublic
Actions

Authored by antonanikin on Dec 14 2016, 3:59 AM.

Details

Reviewers
kfunk
Group Reviewers
KDevelop
Commits
R32:2c04b1a85078: QtHelp page loading fix
Summary

This patch fixes processing of empty requests, which is necessary for successful loadFinished() signal emitting by corresponding QWebView.

Test Plan

Tested on master branch with Qt 5.7.0

Diff Detail

Repository
R32 KDevelop
Branch
master
Lint
No Linters Available
Unit
No Unit Test Coverage
antonanikin updated this revision to Diff 8993.Dec 14 2016, 3:59 AM
antonanikin retitled this revision from to QtHelp fixes.
antonanikin updated this object.
antonanikin edited the test plan for this revision. (Show Details)
antonanikin added a reviewer: KDevelop.
antonanikin added a subscriber: kdevelop-devel.
kfunk requested changes to this revision.Dec 14 2016, 8:51 AM
kfunk added a reviewer: kfunk.
kfunk added a subscriber: kfunk.
kfunk added inline comments.
documentation/qthelp/qthelpnetwork.h
53

This hunk looks good to me . +1

60

Do you have more information about that issue? A bug report maybe? Does it only happen for Qt 5.7.0?

Now we're always patching contents of .css files here, regardless where the .css file came from. Not a good idea I think.

This revision now requires changes to proceed.Dec 14 2016, 8:51 AM
antonanikin added inline comments.Dec 15 2016, 3:39 AM
documentation/qthelp/qthelpnetwork.h
60

Does it only happen for Qt 5.7.0?

No, it also happens on ubuntu 16.04 with Qt 5.5.1 - CSS images are not displayed.

Now we're always patching contents of .css files here, regardless where the .css file came from. Not a good idea I think.

Ok, we can move this to QtHelpDocumentation::setUserStyleSheet() method. Or don't fix this issue - now we lost some small images like arrows, not so critical.

antonanikin added inline comments.Dec 15 2016, 7:22 AM
documentation/qthelp/qthelpnetwork.h
60

we can move this to QtHelpDocumentation::setUserStyleSheet() method

Original offline.css contains 10 (for Qt 5.7.0) wrong lines with relative paths for images. I think it not right to add all such fixes into setUserStyleSheet() method. Therefore we have such variants:

  1. Keep new CSS-patching as is.
  2. Completely remove this code, if it looks as something shady, and push only zero-length content fix.

What are you think about it?

kfunk added inline comments.Dec 16 2016, 11:27 PM
documentation/qthelp/qthelpnetwork.h
60

If it's just offline.css, can we try to a more exact matching on the fileName()? So we can be sure we only replace stuff in the right file.

60

+1 on pushing the zero-length content fix as -is.

antonanikin updated this revision to Diff 9103.Dec 17 2016, 5:45 AM
antonanikin edited edge metadata.

remove CSS patching

antonanikin updated this object.Dec 17 2016, 5:46 AM
antonanikin edited edge metadata.
antonanikin retitled this revision from QtHelp fixes to QtHelp page loading fix.
Closed by commit R32:2c04b1a85078: QtHelp page loading fix (authored by antonanikin). · Explain WhyDec 17 2016, 5:51 AM
This revision was automatically updated to reflect the committed changes.

Revision Contents
Changeset List

PathPackages
Mdocumentation/qthelp/qthelpnetwork.h (14 lines)

Diff 8993

View Options

documentation/qthelp/qthelpnetwork.h

Loading...