Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Multiple Bootstrap related UI tweakings #94

Merged
merged 1 commit into from Jan 13, 2015

Conversation

micolous
Copy link
Contributor

  • Started working on getting some more info about the current talk in the
    navbar.
  • Removed HTML injection issues from the event feeds, making them all use
    the "generated" tag instead.
  • Disallowed HTML in the event feed JSON output generator.
  • Navbar is now white.
  • Add LCA logo.
  • Remove padding between elements.
  • Fix URLs for Zookeepr source.

- Started working on getting some more info about the current talk in the
  navbar.

- Removed HTML injection issues from the event feeds, making them all use
  the "generated" tag instead.

- Disallowed HTML in the event feed JSON output generator.

- Navbar is now white.

- Add LCA logo.

- Remove padding between elements.
@mithro mithro added this to the LCA 2015 milestone Jan 12, 2015
@@ -177,7 +177,7 @@ def main(argv):
else:
sys.stderr.write('Event %r does not have a End time or Duration, dropped.\n' % item['Title'])
continue
if 'url' in item:
if 'URL' in item:
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Case change?

Should this be if 'URL' in item or 'url' in item or something?

Maybe just lower all the keys?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's tracking the current version of Zookeepr, which has some major discrepancies in the keys.

There's multiple issues with the Zookeepr feeds which I've reported in the past:

There's also no schema for the JSON feed which has been broken before without warning. In part this is because there is no schema for this data.

Maybe poking @iseppi into 1) having a published schema for Zookeepr and 2) actually sticking to it would be the best way forward from here.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Lets just work around this it for now. This tool is already doing lots of other horrible munding.

Just lower all the keys.

@mithro
Copy link
Member

mithro commented Jan 12, 2015

Still two open things here;

  • Lower all the keys
  • Confirm the webchat thing prevents leaving the page when logged in
  • Why are you removing all the formatting things?

@micolous
Copy link
Contributor Author

There's more than case sensitivity that is an issue for the Zookeepr schema -- unfortunately is not that simple.

The webchat blocks navigation if logged in, but not if it is not logged in.

The formatting things are replaced with a much saner implementation in JavaScript directly, and now calls .text instead of .html. This means that the event feed no longer has potential for HTML injection issues, because we don't actually bother to sanitise that input data.

@mithro
Copy link
Member

mithro commented Jan 13, 2015

There's more than case sensitivity that is an issue for the Zookeepr schema -- unfortunately is not that simple.

Lets worry about getting Zookeepr fixed another time.

The formatting things are replaced with a much saner implementation in JavaScript directly, and now calls .text instead of .html. This means that the event feed no longer has potential for HTML injection issues, because we don't actually bother to sanitise that input data.

We trust data from zookeepr and we generate ourselves. We also have no "user data" to steal, so injection isn't really an issue. I'm going to merge this, but we should probably look at rolling back this bit.

mithro added a commit that referenced this pull request Jan 13, 2015
Multiple Bootstrap related UI tweakings
@mithro mithro merged commit 59f7e99 into timvideos:master Jan 13, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants