Fix popup code

[joshblum]

Its super hard to read/change anything right now with html and logic mixed. I think using a mustache template or just jquery to build the dom elements would make it much easier to read and maintain

[joshblum]

Partially fixed by d0274046e908afecb09b0a09378b1c5ab7374717

Need to fix a few more to use the new method

[amyxzhang]

hm so the reason why before I was putting the css directly on the tags as opposed to adding it as a separate style sheet was because I was under the impression that this would make it certain that the styles would not be overridden. If you've found a way that still gets it working then I guess it's fine - just putting this out there.

[joshblum]

I think that we're ok since we drop the !important tag on everything http://w3schools.invisionzone.com/index.php?showtopic=41387

According to the post the only thing that would have precedence over this would be inline style with !important which seems very unlikely (unless a site knew our id/class names and did this maliciously)

[karger]

Ask Lea about best practice

-------- Original message --------

From: Amy Zhang notifications@github.com

Date:03/22/2015 7:36 PM (GMT-05:00)

To: haystack/eyebrowse-chrome-ext eyebrowse-chrome-ext@noreply.github.com

Cc:

Subject: Re: [eyebrowse-chrome-ext] Fix popup code (#13)

hm so the reason why before I was putting the css directly on the tags as opposed to adding it as a separate style sheet was because I was under the impression that this would make it certain that the styles would not be overridden. If you've found a way that still gets it working then I guess it's fine - just putting this out there. — Reply to this email directly or view it on GitHub.

[karger]

you could reduce the odds of collisions by namespacing all the classes eg eyebrowse- prefix.

On 3/22/2015 7:58 PM, Joshua Blum wrote:

I think that we're ok since we drop the |!important| tag on everything http://w3schools.invisionzone.com/index.php?showtopic=41387

According to the post the only thing that would have precedence over this would be inline style with |!important| which seems very unlikely (unless a site knew our id/class names and did this maliciously)

— Reply to this email directly or view it on GitHub https://github.com/haystack/eyebrowse-chrome-ext/issues/13#issuecomment-84734808.

[joshblum]

yup we already prefix all the identifiers :)

https://github.com/haystack/eyebrowse-chrome-ext/blob/master/css/prompt.css

[joshblum]

closed by 6ce3ed6926a0a879896fd54f3df430e765d4570a and 58775760b25fac543fa088037a105b1e1bd6e50a @amyxzhang can you verify everything is ok before we push? the changes were kinda extensive. once its ok we can actually close this and #22

[amyxzhang]

I'm having some issues:

Uncaught ReferenceError: _ is not defined common.js:119 
createEscapercommon.js:128 (anonymous function)

Not sure if this is causing it but the bubbles are no longer showing up (I changed the server side endpoint to /ext/bubbleInfo)

Also depending on the page, I get the following failed call: GET http://stackoverflow.com/questions/20458154/%3C%=%20user.pic_url%20%%3E 400 (Bad Request)

I think I'm also getting errors when trying to populate the bulletin, chat, and/or active users window when clicking the eye:

Error in response to tabs.query: ReferenceError: userUrl is not defined
    at eval (eval at <anonymous> (chrome-extension://fgpkoninflamokjngohkmkgkofolecbn/libs/underscore.min.js:30:350), <anonymous>:6:3)
    at Function.b.template (chrome-extension://fgpkoninflamokjngohkmkgkofolecbn/libs/underscore.min.js:30:400)
    at Backbone.View.extend.render (chrome-extension://fgpkoninflamokjngohkmkgkofolecbn/js/popup.js:99:26)
    at null.<anonymous> (chrome-extension://fgpkoninflamokjngohkmkgkofolecbn/js/popup.js:128:38)
    at Function.b.each.b.forEach (chrome-extension://fgpkoninflamokjngohkmkgkofolecbn/libs/underscore.min.js:11:39)
    at r.(anonymous function) [as each] (chrome-extension://fgpkoninflamokjngohkmkgkofolecbn/libs/backbone.min.js:25:314)
    at Backbone.View.extend.render (chrome-extension://fgpkoninflamokjngohkmkgkofolecbn/js/popup.js:124:25)
    at populateActiveUsers (chrome-extension://fgpkoninflamokjngohkmkgkofolecbn/js/popup.js:613:31)
    at Object.callback (chrome-extension://fgpkoninflamokjngohkmkgkofolecbn/js/popup.js:360:13)
    at Backbone.View.extend.render (chrome-extension://fgpkoninflamokjngohkmkgkofolecbn/js/popup.js:351:21)
    at Backbone.View.extend.initialize (chrome-extension://fgpkoninflamokjngohkmkgkofolecbn/js/popup.js:344:14)
    at g.View (chrome-extension://fgpkoninflamokjngohkmkgkofolecbn/libs/backbone.min.js:33:382)
    at new d (chrome-extension://fgpkoninflamokjngohkmkgkofolecbn/libs/backbone.min.js:38:76)
    at HTMLDocument.<anonymous> (chrome-extension://fgpkoninflamokjngohkmkgkofolecbn/js/popup.js:888:20)
    at k (chrome-extension://fgpkoninflamokjngohkmkgkofolecbn/libs/jquery-1.8.2.min.js:2:16920)
    at Object.l.fireWith [as resolveWith] (chrome-extension://fgpkoninflamokjngohkmkgkofolecbn/libs/jquery-1.8.2.min.js:2:17707)

[amyxzhang]

also for the bubbleInfo() call, it's using user.csrf. if a user is logged in via a cookie after re-installing the extension, this value needs to be taken from the server and stored (which is what happens when a user logs in explicitly on the extension: https://github.com/haystack/eyebrowse-chrome-ext/blob/master/js/popup.js#L259). otherwise the field will be blank and the call will have a 403. You could do a check right before and set it or just fix that case. I realize this is why I haven't been seeing bubbles recently.

[joshblum]

ok so the error:

Uncaught ReferenceError: _ is not defined common.js:119 
createEscapercommon.js:128 (anonymous function)

should be fixed by 4612c08fc7248e7cdda116f0141f950b684d3906. i was testing with a page with underscore library as part of the resources i think :/

And i think that the CSRF stuff should be fixed by 0cf75e9d95d445a2e6c905269af4cd29abb546e0, so basically the isLoggedIn state and the CSRF being present should be tied together now.

[amyxzhang]

I'm still getting the other issues I mentioned. I do see the bubble now but it doesn't look right:

[amyxzhang]

As for the window, the error is now:

Error in response to tabs.query: ReferenceError: pic_url is not defined
    at eval (eval at <anonymous> (chrome-extension://fgpkoninflamokjngohkmkgkofolecbn/libs/underscore.min.js:30:350), <anonymous>:8:3)
    at Function.b.template (chrome-extension://fgpkoninflamokjngohkmkgkofolecbn/libs/underscore.min.js:30:400)
    at Backbone.View.extend.render (chrome-extension://fgpkoninflamokjngohkmkgkofolecbn/js/popup.js:99:26)
    at null.<anonymous> (chrome-extension://fgpkoninflamokjngohkmkgkofolecbn/js/popup.js:128:38)
    at Function.b.each.b.forEach (chrome-extension://fgpkoninflamokjngohkmkgkofolecbn/libs/underscore.min.js:11:39)
    at r.(anonymous function) [as each] (chrome-extension://fgpkoninflamokjngohkmkgkofolecbn/libs/backbone.min.js:25:314)
    at Backbone.View.extend.render (chrome-extension://fgpkoninflamokjngohkmkgkofolecbn/js/popup.js:124:25)
    at populateActiveUsers (chrome-extension://fgpkoninflamokjngohkmkgkofolecbn/js/popup.js:624:31)
    at Object.callback (chrome-extension://fgpkoninflamokjngohkmkgkofolecbn/js/popup.js:360:13)
    at Backbone.View.extend.render (chrome-extension://fgpkoninflamokjngohkmkgkofolecbn/js/popup.js:351:21)
    at Backbone.View.extend.initialize (chrome-extension://fgpkoninflamokjngohkmkgkofolecbn/js/popup.js:344:14)
    at g.View (chrome-extension://fgpkoninflamokjngohkmkgkofolecbn/libs/backbone.min.js:33:382)
    at new d (chrome-extension://fgpkoninflamokjngohkmkgkofolecbn/libs/backbone.min.js:38:76)
    at HTMLDocument.<anonymous> (chrome-extension://fgpkoninflamokjngohkmkgkofolecbn/js/popup.js:888:20)
    at k (chrome-extension://fgpkoninflamokjngohkmkgkofolecbn/libs/jquery-1.8.2.min.js:2:16920)
    at Object.l.fireWith [as resolveWith] (chrome-extension://fgpkoninflamokjngohkmkgkofolecbn/libs/jquery-1.8.2.min.js:2:17707)

[amyxzhang]

And there's still a bad call: GET http://127.0.0.1:8000/live_stream/%3C%=%20user.pic_url%20%%3E 404 (NOT FOUND)

[joshblum]

haha you're a few steps ahead of me :p

The bad call should be fixed by: 44e93044bc2622558c54950213a7a5d651464f1d The userUrl problem should be fixed by: 0d73d7074c14b04587414af49ea98451c1604586 Can you give the the url you're seeing the bubble on?

[amyxzhang]

I think it was a stackoverflow page.

But instead of bubbles, I'm now getting:

prompt.js:54 Uncaught ReferenceError: active_users is not defined

And the window loads properly now with no errors but in the active users window, instead of displaying usernames displays the word "undefined"

[joshblum]

Ok active_users fix: 9384f47cb26ef253090e618a984e959de5843724 undefined fix: cc15ca531f617168cf2e38100c0f9effe321c756 bubble prompt fix: b183211c82fb617c268f399b48e806ab50b24471

With a server push for the bubbleInfo endpoint i think we should be good to go :)

[amyxzhang]

yay looks a lot better now! one quick issue - the X button on the bubble seems to be clickable on just a small part of it (the top part). for most of the X (at least for me) clicking does nothing. It seems to be covered up by another div?

[amyxzhang]

hm also just comparing stackoverflow and eyebrowse.csail I'm seeing the X is larger in stackoverflow and everything seems to be shifted down in the bubble for eyebrowse.csail. if these are CSS issues, we can go ahead and push but keep an eye out for more style rules to set. I would be careful about any tags you are using - make sure to give every single tag an id and set all basic style rules for each of them. For ex I noticed a naked tag in the bubble.

yup the x was being covered, this should take care of it: b183211c82fb617c268f399b48e806ab50b24471

As for the tags the, .eyebrowse-reset class should be inherited among all of the elements and set default values (and have higher priority than the page defaults)

[karger]

i know we've been abandoning iframes in some place, but they do provide a good insulation from css....

On 03/27/2015 01:24 PM, Amy Zhang wrote:

hm also just comparing stackoverflow and eyebrowse.csail I'm seeing the X is larger in stackoverflow and everything seems to be shifted down in the bubble for eyebrowse.csail. if these are CSS issues, we can go ahead and push but keep an eye out for more style rules to set. I would be careful about any tags you are using - make sure to give every single tag an id and set all basic style rules for each of them. For ex I noticed a naked tag in the bubble.

— Reply to this email directly or view it on GitHub https://github.com/haystack/eyebrowse-chrome-ext/issues/13#issuecomment-87019219.

[amyxzhang]

I'm getting some odd formatting when there is a comment/chat:

[amyxzhang]

Another page, this time without comments:

[joshblum]

What is the link for the second page so i can reproduce?

also I can't remember the reasons for switching from iframes? https?

[amyxzhang]

i think if you go to any page with more than 1 person in the bubble, you might this. This was just on my personal MIT website: http://people.csail.mit.edu/axz

We switched from iframe for a number of reasons - broken iframes leading to problems, privacy concerns brought up by various people, cleanliness of the code organization, less reliance on the server.

I think if we can get it to look good for most websites, with some weird edge cases, it's fine.

On Fri, Mar 27, 2015 at 4:45 PM, Joshua Blum notifications@github.com wrote:

What is the link for the second page so i can reproduce?

also I can't remember the reasons for switching from iframes? https?

— Reply to this email directly or view it on GitHub https://github.com/haystack/eyebrowse-chrome-ext/issues/13#issuecomment-87085211 .

Amy X. Zhang | Ph.D. student at MIT CSAIL | http://people.csail.mit.edu/axz | @amyxzh

[joshblum]

oh right we talked about this recently haha

these bugs are my fault with porting to template code, we should see if we run into more issues after though and decide if we want to move back.