[Corona] Adding correct handling for external resources(css/js)

Review Request #419 — Created March 1, 2016 and submitted

smillernl
Lunr
fix/external_html_resources
f5134d8...
lunr
Corona: Adding correct handling for external resources(css/js)


  • 0
  • 0
  • 4
  • 0
  • 4
Description From Last Updated
tardypad
  1. 
      
  2. src/Lunr/Corona/HTMLView.php (Diff revision 1)
     
     
     
     
     
     
     

    This looks wrong to me
    a local stylesheet can be used starting with "http://" and such (that's why the str_replace is for)
    What you better check if the file exists I think

    1. The str_replace is to get an absolute path from a provided relative path. As for stylesheet names starting with a protocol defenition, that's just a shitty idea and shouldn't be encouraged by tools.

    2. forget my comment about local stylesheet starting with "http://", this part is not included in the request base_path
      It wouldn't work anyway right now

      I don't get your point of the stylesheets with protocol definition
      Something must be missing in your sentence

      I would group this "is_external" check in a different function though
      since you reuse the same for the javascript files

  3. 
      
smillernl
tardypad
  1. 
      
  2. src/Lunr/Corona/Tests/HTMLViewHelpersTest.php (Diff revision 2)
     
     
     
     
     
     
     
     
     
     

    duplicate from above. remove

  3. src/Lunr/Corona/Tests/HTMLViewHelpersTest.php (Diff revision 2)
     
     
     
     
     
     
     
     
     
     

    duplicate from above. remove

  4. 
      
smillernl
pprkut
  1. 
      
  2. src/Lunr/Corona/HTMLView.php (Diff revision 3)
     
     

    this looks suspiciously like a tab indent

    1. It's not though, unless something on the server converted it.

    2. Interesting. Fair enough then.

  3. 
      
pprkut
  1. Ship It!
  2. 
      
smillernl
Review request changed

Status: Closed (submitted)

Change Summary:

Merged into master

Loading...