[CLOSED] Fix #10786 - Allow QuickView image preview for arbitrary URLs

[core-ai-bot]

Issue by humphd
Tuesday Mar 24, 2015 at 03:09 GMT
Originally opened as adobe/brackets#10788

---

This leverages the img element's load and error events to try and load arbitrary images. It also expands the list of known image extensions to include .bmp, which browsers can display.

---

humphd included the following code: https://github.com/adobe/brackets/pull/10788/commits

[core-ai-bot]

Comment by humphd
Friday Mar 27, 2015 at 16:27 GMT

---

I've updated this with the suggestion made to examine extensions.

[core-ai-bot]

Comment by MarcelGerber
Friday Mar 27, 2015 at 20:57 GMT

---

Could you take a look at the data provided by language.json/LanguageManager and see if you can use them? image is particularly interesting for whitelist (please concat "svg" yourself), audio and binary for blacklist (concat the ones you already have, plus maybe also "rss", "php").

[core-ai-bot]

Comment by humphd
Monday Apr 13, 2015 at 19:22 GMT

---

I've rewritten this to use LanguageManager, good suggestion.

[core-ai-bot]

Comment by humphd
Saturday Apr 25, 2015 at 17:08 GMT

---

This is ready for review.

[core-ai-bot]

Comment by nethip
Thursday Apr 30, 2015 at 09:16 GMT

---

@humphd Can you add some unit tests, if possible? Also run smoke tests with these changes, especially the one relating to editor and Extensions.

[core-ai-bot]

Comment by nethip
Thursday Apr 30, 2015 at 09:31 GMT

---

LGTM

[core-ai-bot]

Comment by redmunds
Thursday Apr 30, 2015 at 14:55 GMT

---

The original Quick View implementation had support for arbitrary URLs, but it was removed due to concerns about performance (i.e. downloading external files on mousemove!). The code has been optimized since then so it now does not try to parse code in editor until mouse hasn't moved for 350ms, but people with a slow network or no network connection might see problems.

Be sure to test this with no internet connection!

You should consider:

1. Making this a preference. It's probably OK to default to ON as long as there's a way to turn it OFF.
2. Caching images. No need to re-download same image over and over. Maybe this already takes advantage of browser cache. Then there's the opposite problem of a stale cache :)

[core-ai-bot]

Comment by humphd
Thursday Apr 30, 2015 at 19:10 GMT

---

I've updated this again. New commit adds:

• pref for enabling/disabling extensionless previews for URLs (on by default). Let me know if this is how you want such a pref to be done.
• tests for everything that seemed obvious to me about what this patch is doing.

Regarding@redmunds' questions. First, with no network, you end up in the error path (net::ERR_INTERNET_DISCONNECTED exception or the like trying to load the resource) and it does the right thing. I think cache is best handled by the browser. Trying to build something more intelligent than what is already there for free seems like a waste of effort to me.

[core-ai-bot]

Comment by redmunds
Tuesday May 05, 2015 at 00:03 GMT

---

In general, this looks good. Be sure to turn on JSLint as I am seeing some JSLint errors in QuickView/main.js. Also, there is a merge conflict that needs to be resolved. Just 1 other comment.

[core-ai-bot]

Comment by humphd
Tuesday May 05, 2015 at 01:42 GMT

---

@redmunds thanks for the review. I've rebased and pushed fixes. See if this approach satisfies what you wanted with the pref saving and startup.

[core-ai-bot]

Comment by redmunds
Wednesday May 06, 2015 at 00:57 GMT

---

@humphd

See if this approach satisfies what you wanted with the pref saving and startup.

Looks good.

[core-ai-bot]

Comment by humphd
Friday May 08, 2015 at 22:06 GMT

---

Great, can someone land this then?

[core-ai-bot]

Comment by nethip
Tuesday May 26, 2015 at 06:24 GMT

---

Thanks@humphd for this fix