Closed Bug 961406 Opened 11 years ago Closed 11 years ago

Goggles doesn't add class attributes to <html> element

Categories

(Webmaker Graveyard :: X-Ray Goggles, defect)

Product:
Webmaker Graveyard
Component:
X-Ray Goggles
Platform:
x86
macOS
Type:
defect
Priority:
Not set
Severity:
normal

Tracking

(Not tracked)

Status:
RESOLVED FIXED

People

(Reporter: brett, Assigned: michiel)

References

(
URL
)

Details

Attachments

(1 file)

https://github.com/mozilla/goggles.webmaker.org/pull/91
55 bytes, text/x-github-pull-request
humph
: review+
Details | Review
Happily we can now remix sites with odd markup like NYtimes - however, if you publish a work on NYtimes, you see odd CSS: http://etherworks.makes.org/goggles/etherworkss-remix-of-the-new-york-times-breaking-news-world-news-amp-multimedia-2 Is there any simple solution that could be done here? We discussed in IRC but wanted to file a bug as NYTimes is often the "go to page" for the goggles.
* This is a blocker for me publishing a Webmaker blost post announcement about the "new and improved" Goggles. So CCing myself here.
* Is this a blocker for letting people about our new and improved Goggles? I ask because: a) I have a Webmaker blog post waiting on this. b) We talked about mentioning it in today's 2pm ET All Hands meeting
Flags: needinfo?(pomax)
From irc, I think we're cooked on this: 11:28 < Pomax> nytimes does some CORS on its own resources 11:29 < Pomax> so if the request isn't from nytimes.com, but makes.org, no dice 11:29 < Pomax> unless we start faking the entire network architecture through curl.js or something, I doubt we can get that to work 11:29 < humph> in which case 11:29 < humph> we aren't going to fix this 11:29 < Pomax> bottom line: nytimes REALLY doesn't want to be public data. just publically visible 11:29 < humph> s/aren't/can't/ 11:29 < humph> OpenMatt: ^^ 11:29 < humph> RESOLVED CRAPPY 11:30 < Pomax> I wonder if we can somehow steal JSON.stringify(document.styleSheets) 11:30 < cade> I think 11:30 < Pomax> then strip all script and links 11:30 < Pomax> though that would turn a website into a "picture of a website" much more than it does atm I don't know how we resolve this, since they are basically not going to honour requests from us at makes.org for that extra content. I suggest we resolve this as WONTFIX/CANTFIX, unless someone else has ideas?
nytimes.com only has an issue with some vertical space, but otherwise is the same. cbc.ca I can't see any obvious differences, so I'd say show it off because being able to remix these sites is super cool. And then NYTimes.com has some nifty code that relies on requests for resources coming from their own server, which also demonstrates the limitations of a technology like goggles. Which is a lot better than goggles not even working on NYtimes.com My vote is to show it off, and then as an actual issue resolvement, this is something we should probably not try to solve either. If a site has CORS that prevents their resources from loading except on their own site, then that's just a thing that happens on the internet. goggles currently can't, and probably never should, get around that kind of content security policy set up by the content owner.
Flags: needinfo?(pomax)
URL: http://nytimes.com
Summary: Investigate broken NYTimes.com published make → Goggles doesn't add class attributes to <html> element
turns out that the page works just fine, IF goggles copies over the attributes on the <html> element, which it seems to be ignoring completely at the moment. So: it works, it just doesn't do an extra check on whether there is data on the html element.
pretty sure our culprit ihttps://github.com/mozilla/goggles.webmaker.org/blob/master/lib/overrides/uproot.js#L91, which creates a new document with "clean" doctype and html, then injects the document.innerHTML We can probably rewrite this to use document.querySelector("html").outerHTML instead of building "<html"> + document innerhtml + "</html>".
uses outerHTML on document.querySelector("html"), which is a guaranteed fix on modern browsers and may or may not work on legacy versions of IE and/or Safari
Attachment #8362622 - Flags: review?(david.humphrey)
Comment on attachment 8362622 [details] [review] https://github.com/mozilla/goggles.webmaker.org/pull/91 Awesome work tracking this down and fixing so fast. ^5 Pomax!
Attachment #8362622 - Flags: review?(david.humphrey) → review+
Commit pushed to master at https://github.com/mozilla/goggles.webmaker.org https://github.com/mozilla/goggles.webmaker.org/commit/6fc68b42abd2ae50c7a4433692a3b1624e9b6c74 Merge pull request #91 from Pomax/bug961406 folds the <html> tag into the HTML we take off a page
Status: NEW → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
* Now that this is fixed, I'm going to publish that "What's new on Webmaker" blog post we prepped after Friday's demos
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: