Closed
Bug 659466
Opened 14 years ago
Closed 14 years ago
Fix remaining "#-in-data-URI" issues in test files
Categories
(Core :: General, defect)
Core
General
Tracking
()
RESOLVED
FIXED
mozilla7
People
(Reporter: dholbert, Assigned: dholbert)
References
Details
Attachments
(1 file)
7.27 KB,
patch
|
bzbarsky
:
review+
|
Details | Diff | Splinter Review |
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
Assignee | ||
Comment 1•14 years ago
|
||
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
Assignee | ||
Comment 2•14 years ago
|
||
(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()".
Assignee | ||
Comment 3•14 years ago
|
||
> 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.
Assignee | ||
Comment 4•14 years ago
|
||
(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)
Assignee | ||
Comment 5•14 years ago
|
||
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 6•14 years ago
|
||
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+
Assignee | ||
Comment 7•14 years ago
|
||
(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.
Assignee | ||
Comment 8•14 years ago
|
||
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
Comment 9•14 years ago
|
||
Status: ASSIGNED → RESOLVED
Closed: 14 years ago
Flags: in-testsuite+
Resolution: --- → FIXED
Whiteboard: fixed-in-cedar
Target Milestone: --- → mozilla7
Assignee | ||
Comment 10•14 years ago
|
||
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.
Description
•