SteamedPears / Code-Review

A web app for reviewing code (and other text), written in node.js using redis as the persistent store.
http://review.steamedpears.com
ISC License
1 stars 2 forks source link

Bookmarklet broken #158

Open cdelahousse opened 11 years ago

cdelahousse commented 11 years ago

http://review.steamedpears.com/bookmarklet/bookmarklet.js is returning a 404.

spratt commented 11 years ago

You're right, because it should be using bookmarklet.min.js. At least, that's what the build script builds.

Also, there's a typo in the build script that outputs it to bookmarket/ instead of bookmarklet/. See: http://review.steamedpears.com/bookmarket/bookmarklet.min.js

spratt commented 11 years ago

Fixed the typo in the build script: http://review.steamedpears.com/bookmarklet/bookmarklet.min.js

cdelahousse commented 11 years ago

Shit. Nor is /bookmarklet/style.css being served.

spratt commented 11 years ago

Yeah, that file doesn't even get built by the build script.

cdelahousse commented 11 years ago

Also, the cors module isn't working.

spratt commented 11 years ago

What makes you say that?

cdelahousse commented 11 years ago

I'm getting this HTTP header/message:

OPTIONS (failed) Origin http://www.lipsum.com is not allowed by Access-Control-Allow-Origin.

spratt commented 11 years ago

Cool. Things to fix.

cdelahousse commented 11 years ago

I'm fixing the CSS now...

cdelahousse commented 11 years ago

@spratt says "Does it work if you build the bookmarklet in dev mode? I think the root cause of this issue was that dev mode builds bookmarklet.js and production builds bookmarklet.min.js"

All the production script does is uglify the bookmarklet.js to a file named bookmarklet.min.js code. The code still runs fine. The issue I'm getting is that the server is no longer sending an OPTIONS header.

cdelahousse commented 11 years ago

Hold on. I'll retest locally.

spratt commented 11 years ago

I appreciate everything you're saying here, but the context of that question was a line of code that specifically references the uglified version.

cdelahousse commented 11 years ago

Even testing locally on the unminified build of the bookmarklet against my local install, I still get the same OPTION error.

spratt commented 11 years ago

Look: that question is about a GET request for bookmarklet.min.js when you may have only built bookmarklet.js. The CORS issue is separate.

cdelahousse commented 11 years ago

Look: that's clearer. Thanks.