From kde-panel-devel Tue Jun 30 17:36:21 2015 From: "David Edmundson" Date: Tue, 30 Jun 2015 17:36:21 +0000 To: kde-panel-devel Subject: Re: Review Request 124204: Use proper relative paths to import js code Message-Id: <20150630173621.7067.27052 () mimi ! kde ! org> X-MARC-Message: https://marc.info/?l=kde-panel-devel&m=143568580506759 MIME-Version: 1 Content-Type: multipart/mixed; boundary="--===============1025921881744595740==" --===============1025921881744595740== Content-Type: multipart/alternative; boundary="===============3022366310798411675==" --===============3022366310798411675== MIME-Version: 1.0 Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: 7bit > On June 29, 2015, 7:57 a.m., David Edmundson wrote: > > Was this causing an error? > > Kevin Ottens wrote: > Yes, most of the applets (see my other patches) were broken after my update this week-end. I didn't update in a while previously. I noted that the applets were inconsistent some using import "foo.js" (all broken for me) and some using import "../code/foo.js" (all working for me). > > Because of the inconsistency I went on making it consistent again picking the one which worked in practice. > > Note I'm testing with Qt 5.5 on my end. I have Qt5.5 here too; though maybe a month old. I'd rather we have some idea why it works for some and not your case, otherwise we have a risk that our 3rd party applets/existing stable releases are going to suddenly break and we'll have ignored the advanced warning. From what I can tell in PackageUrlInterceptor, when it tries searching for any relative URL inside something loaded from a package, it will look in every contentsPrefixPaths() i.e each registered folder in a plasma package structure; ui, code, config for that filename. see packageurlinterceptor:127 it's not just for package:// urls. I don't understand why we do that, seems a bit weird to me, but we should keep compatibility if we've had that in previous releases. I'll update my Qt to see if that changes anything. - David ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/124204/#review81844 ----------------------------------------------------------- On June 29, 2015, 7:15 a.m., Kevin Ottens wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://git.reviewboard.kde.org/r/124204/ > ----------------------------------------------------------- > > (Updated June 29, 2015, 7:15 a.m.) > > > Review request for Plasma. > > > Repository: bluedevil > > > Description > ------- > > Use proper relative paths to import js code > > > Diffs > ----- > > src/applet/package/contents/ui/BluetoothApplet.qml 4e8b9648983edd2c8db242515464ae22d1e6fbe6 > src/applet/package/contents/ui/CompactRepresentation.qml 7085a7ec82fa88cb2d8e1a58847215845f3d5658 > > Diff: https://git.reviewboard.kde.org/r/124204/diff/ > > > Testing > ------- > > > Thanks, > > Kevin Ottens > > --===============3022366310798411675== MIME-Version: 1.0 Content-Type: text/html; charset="utf-8" Content-Transfer-Encoding: 7bit

This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/124204/

On June 29th, 2015, 7:57 a.m. UTC, David Edmundson wrote:

Was this causing an error?

On June 29th, 2015, 8:48 a.m. UTC, Kevin Ottens wrote:

Yes, most of the applets (see my other patches) were broken after my update this week-end. I didn't update in a while previously. I noted that the applets were inconsistent some using import "foo.js" (all broken for me) and some using import "../code/foo.js" (all working for me).
Because of the inconsistency I went on making it consistent again picking the one which worked in practice.
Note I'm testing with Qt 5.5 on my end.

I have Qt5.5 here too; though maybe a month old.
I'd rather we have some idea why it works for some and not your case, otherwise we have a risk that our 3rd party applets/existing stable releases are going to suddenly break and we'll have ignored the advanced warning.
From what I can tell in PackageUrlInterceptor, when it tries searching for any relative URL inside something loaded from a package, it will look in every contentsPrefixPaths() i.e each registered folder in a plasma package structure; ui, code, config for that filename. 
see packageurlinterceptor:127 it's not just for package:// urls.
I don't understand why we do that, seems a bit weird to me, but we should keep compatibility if we've had that in previous releases.
I'll update my Qt to see if that changes anything.

- David

On June 29th, 2015, 7:15 a.m. UTC, Kevin Ottens wrote:

Review request for Plasma.

By Kevin Ottens.

Updated June 29, 2015, 7:15 a.m.

Repository: bluedevil

Description

Use proper relative paths to import js code

Diffs

src/applet/package/contents/ui/BluetoothApplet.qml (4e8b9648983edd2c8db242515464ae22d1e6fbe6)
src/applet/package/contents/ui/CompactRepresentation.qml (7085a7ec82fa88cb2d8e1a58847215845f3d5658)

--===============3022366310798411675==-- --===============1025921881744595740== Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: base64 Content-Disposition: inline X19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX18KUGxhc21hLWRl dmVsIG1haWxpbmcgbGlzdApQbGFzbWEtZGV2ZWxAa2RlLm9yZwpodHRwczovL21haWwua2RlLm9y Zy9tYWlsbWFuL2xpc3RpbmZvL3BsYXNtYS1kZXZlbAo= --===============1025921881744595740==--