Closed Bug 659466 Opened 14 years ago Closed 14 years ago

Fix remaining "#-in-data-URI" issues in test files

Categories

(Core :: General, defect)

defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla7

People

(Reporter: dholbert, Assigned: dholbert)

References

Details

Attachments

(1 file)

Bug 658949 (intentionally) broke data URIs that have "#" in them, basically. "patch 0" on that bug fixed all the test failures that resulted from this. I'm now doing an audit for remaining instances of this (that aren't causing test failures for whatever reason). I don't anticipate that there will be many of these, but I've already found a few.* FWIW, I'm finding these by running > grep -r -A6 "data:" across m-c, and then searching through the results for instances of "#". Hopefully this is sensitive enough to catch most or all instances of this. * so far, I've found this in window_activation.xul & test_activation.xul
Attached patch fixSplinter Review
Here's the fix for all the instances I found. Haven't tested it yet. Notes on the touched files & why they weren't failing after bug 658949 landed: ===== > dom/tests/mochitest/chrome/test_activation.xul > dom/tests/mochitest/chrome/window_activation.xul (still need to look into these two xul files) > gfx/thebes/gfxFont.cpp Just fixing a sample data URI given in a comment. > layout/reftests/bugs/605138-1-ref.html > layout/reftests/bugs/605138-1.html In this case, bug 658949 broke the same data URI in the testcase & reference case, so they continued rendering the same. Makes sense. > layout/style/test/test_bug405818.html In this case, the test just does a string-comparison on some data URIs that have # characters in them, which still works fine even after bug 658949 busted the semantics of those URIs.
Assignee: nobody → dholbert
Status: NEW → ASSIGNED
(In reply to comment #1) > ===== > > dom/tests/mochitest/chrome/test_activation.xul > > dom/tests/mochitest/chrome/window_activation.xul > > (still need to look into these two xul files) (note: window_activation.xul is a helper file for test_activation.xul.) Hmm, I see... this test wasn't affected because it doesn't actually check anything right now. :) The test currently has lines 4 lines like this: > ok(getComputedStyle(document.getElementById("box"), "").backgroundColor, "rgb(0, 0, 255)"); Note that the above line uses "ok()", which just evaluates the truthiness of its first argument. So in current mozilla-central, as long as .backgroundColor is *anything* truthy, the test passes. It wants to be using "is()", not "ok()".
> It wants to be using "is()", not "ok()". ...and once I fix that (and apply this bug's hash-fixing-patch), the test fails. I'll file a separate bug on that, after making absolutely sure it's unrelated to this hash-in-data-URI business.
(In reply to comment #3) > I'll file a separate bug on that (filed bug 659522 on fixing up the ineffective checks in window_activation.xul)
Comment on attachment 534922 [details] [diff] [review] fix Requesting review. See comment 1 & 2 for an overview of the changed files and why the existing testcases aren't currently causing orange.
Attachment #534922 - Flags: review?(bzbarsky)
Comment on attachment 534922 [details] [diff] [review] fix r=me The fact that some of these tests were passing is sort of sadmaking. :( Any way we can change them to fail in the future if people do this sort of thing?
Attachment #534922 - Flags: review?(bzbarsky) → review+
(In reply to comment #6) > Comment on attachment 534922 [details] [diff] [review] [review] > fix > > r=me > > The fact that some of these tests were passing is sort of sadmaking. yeah :-/ (just to clarify: _all_ the tests on _this_ bug here were passing) > Any way we can change them to fail in the future if people do this sort of thing? The *_activation.xul tests are just broken, and once they're fixed up, they'll presumably depend on getting colors from their stylesheet in order to pass. Bug 659522 covers that. I'll see about adding some minor bit of color to the data-URI-stylesheets in 605138-1.html. And I suppose we could examine the computed style of some element in test_bug405818.html and assert that it's what we expect.
Landed fix in cedar: http://hg.mozilla.org/projects/cedar/rev/39f04e61e994 I added an 'outline' declaration to the data-URI-stylesheet in 605138-1.html, whereas in the reference case, I added this declaration inside a new <style> chunk. So now if the data URI fails to parse correctly, that test will fail. I also added a computed-style check to test_bug405818.html to be sure that the data-URI-stylesheet there loads correctly.
OS: Linux → All
Hardware: x86_64 → All
Whiteboard: fixed-in-cedar
Status: ASSIGNED → RESOLVED
Closed: 14 years ago
Flags: in-testsuite+
Resolution: --- → FIXED
Whiteboard: fixed-in-cedar
Target Milestone: --- → mozilla7
Pushed to aurora as test/comment-only change: http://hg.mozilla.org/releases/mozilla-aurora/rev/d7fbba849199
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: