Bugzilla – Full Text Bug Listing
| Summary: | IE Browser widget should always encode LocationEvent#location, also for file:/// | ||||||
|---|---|---|---|---|---|---|---|
| Product: | [Eclipse Project] Platform | Reporter: | Markus Keller <markus.kell.r> | ||||
| Component: | SWT | Assignee: | Grant Gayed <grant_gayed> | ||||
| Status: | RESOLVED FIXED | QA Contact: | |||||
| Severity: | normal | ||||||
| Priority: | P3 | CC: | daniel_megert, eclipse.felipe, gheorghe, Silenio_Quarti, tom.schindl | ||||
| Version: | 3.8 | Flags: | gheorghe:
review+
|
||||
| Target Milestone: | 3.8 RC1 | ||||||
| Hardware: | PC | ||||||
| OS: | Windows 7 | ||||||
| Whiteboard: | |||||||
| Bug Depends on: | |||||||
| Bug Blocks: | 367629 | ||||||
| Attachments: |
|
||||||
(In reply to comment #0) > <html><head><base href='file://C:/te%20mp/'></head><body><a > href='fi%20le.html'>Link</a></body></html> Sorry, this is a corner case, since the base href is already wrongly formatted. If I use a correct file:///... URL, then the mixed encoding goes away and the whole location is not encoded (although it should be encoded): <html><head><base href='file:///C:/te%20mp/'></head> <body><a href='fi%20le.html'>Link</a></body></html> The corner case is not that important. This bug is about the generally missing encoding and the missing spec in the API. I think IE.java should replace both "PROTOCOL_FILE + url.replace('\\', '/')" by calls to the Windows API UrlCreateFromPath.
Ping. We should fix bug 368629, but I'd hate to add a workaround just for the IE browser. Created attachment 215591 [details]
Tests and fix (using File#toURI())
I took a stab at fixing this bug and wrote a bunch of tests to be sure I don't break anything.
I was quite confident I had a good fix until I realized I need java.io.File#toURI(), which is only available in Java 1.4.
Apart from that, the attached patch works fine and the tests are green on all major platforms (Win7, Mac, GTK).
Is the CDC-1.0/Foundation-1.0 compatibility still a hard requirement for the whole SWT? Does that restriction make sense in the browser widget?
If yes, is it very hard to use Windows API UrlCreateFromPath instead? Unfortunately, I have no clue about adding new native interface methods.
There's one remaining bug in IE that's probably impossible to fix: Since IE mistreats file URLs as paths, it loses the fragment of a file:/// link (the stuff behind the #). This also occurs in the native Internet Exploder and can be seen when you put this into an html file and then open it & click the link:
<html><head><base href='file:///C:/te%23mp/'></head><body>
<a href='file.html#ref'>Link</a></body></html>
The tests account for this bug.
Silenio, any chance for RC1? Thanks for the test cases. Reviewed with BG, fixed > 20120515, patch: http://git.eclipse.org/c/platform/eclipse.platform.swt.git/commit/bundles?id=3b7e04304effe884e33175739d7c3054dcac5e13 . Thanks, looks good in 4.2 (I20120515-2200), and I assume the 3.8 build input will follow soon. Yes, the change is in our 3.8 stream, but I think our 3.8 build submission failed last night. |
The IE Browser widget should always encode LocationEvent#location, also for "file:///" URLs. Currently, the location is properly encoded for URLs with other protocols (e.g. http:// or foo://), but the file:// protocol is handled specially, and even unusably in some situations. Example: <html><head><base href='file://C:/te%20mp/'></head><body><a href='fi%20le.html'>Link</a></body></html> When you use setText in the ControlExample and then click the link, then the LocationListener events have location "file://C:/te%20mp/fi le.html". Note how the first part of the URL is properly encoded, but the second part is not. Without the <base> element, the location is not encoded for file:// URLs with IE, e.g.: - setUrl("file:///C:/t/sp%20ace/link.html") notifies location "file:///C:/t/sp ace/link.html" (bad), but - setUrl("foo:///C:/t/sp%20ace/link.html" notifies "foo:///C:/t/sp%20ace/link.html" (good). (In reply to bug 305230 comment #3) > I still think LocationEvent#location should specify that the string is an > encoded URI (non-ASCII characters escaped as per RFC 2396). This should really be specified in the API.