Closed Bug 655478 Opened 14 years ago Closed 14 years ago

CSSStyleDeclaration.setProperty shouldn't require the third parameter to be set

Categories

(Core :: DOM: CSS Object Model, defect)

Product:
Core
Component:
DOM: CSS Object Model
Type:
defect
Priority:
Not set
Severity:
normal

Tracking

()

Status:
RESOLVED FIXED
Milestone:
mozilla13

People

(Reporter: rzufall36, Assigned: ayg)

References

(
URL
)

Details

Attachments

(1 file, 1 obsolete file)

Patch v2
2.47 KB, patch
ayg
: review+
Details | Diff | Splinter Review
User-Agent: Mozilla/5.0 (X11; Linux i686) AppleWebKit/534.24 (KHTML, like Gecko) Ubuntu/10.04 Chromium/11.0.696.57 Chrome/11.0.696.57 Safari/534.24 Build Identifier: Mozilla/5.0 (X11; Linux i686; rv:2.0.1) Gecko/20100101 Firefox/4.0.1 I wrote a user script that uses the method setProperty on a CSSStyleDeclaration object, but only with the first two parameters, leaving the third out. Reproducible: Always Steps to Reproduce: 1. Install this script in Greasemonkey http://userscripts.org/scripts/show/101550 2. Navigate to http://blog.fefe.de or https://blog.fefe.de in your browser Actual Results: No searchform as expected appeared and the error console said "Not enough arguments [nsIDOMCSSStyleDeclaration.setProperty]", referring to line 19, where setProperty is used. Expected Results: It should have given the style the default priority and added "float:right" as style, and then (obviously) added the form to the DOM. Please note that the script works just fine in Google Chrome, where it smoothly adds the search form, without even bothering about the missing third parameter in the console.
Chrome treats all parameters to all DOM methods as optional, whether the spec says to do that or not. In DOM 2 CSS (which is what we implement), the third parameter is required. In the proposed CSSOM, the third parameter is optional, but that proposal is in very early stages and I'm not sure we want to be implementing it yet.
Test-case: data:text/html,<!doctype html> <script> document.head.style.setProperty("width", "5px") </script> In mozilla-central, this throws. In IE10 Developer Preview, Chrome 18 dev, and Opera Next 12.00 alpha, it succeeds. This is useful behavior, I think.
Assignee: nobody → ayg
Status: UNCONFIRMED → ASSIGNED
Ever confirmed: true
Attached patch Patch v1 (obsolete) — — Splinter Review
For testing, I just changed one of the existing setProperty()s in test_priority_preservation.html to omit the third argument, leaving the others intact.
Attachment #597466 - Flags: review?(bzbarsky)
Whiteboard: [autoland]
Whiteboard: [autoland] → [autoland-in-queue]
Autoland Patchset: Patches: 597466 Branch: mozilla-central => try Destination: http://hg.mozilla.org/try/pushloghtml?changeset=e5363e1140b7 Try run started, revision e5363e1140b7. To cancel or monitor the job, see: https://tbpl.mozilla.org/?tree=Try&rev=e5363e1140b7
Comment on attachment 597466 [details] [diff] [review] Patch v1 You actually don't need to change the IID, because the C++ side is completely binary-compatible. r=me with that removed
Attachment #597466 - Flags: review?(bzbarsky) → review+
Attached patch Patch v2 — — Splinter Review
Attachment #597466 - Attachment is obsolete: true
Attachment #597541 - Flags: review+
Try run for e5363e1140b7 is complete. Detailed breakdown of the results available here: https://tbpl.mozilla.org/?tree=Try&rev=e5363e1140b7 Results (out of 210 total builds): success: 174 warnings: 22 failure: 14 Builds (or logs if builds failed) available at: http://ftp.mozilla.org/pub/mozilla.org/firefox/try-builds/autolanduser@mozilla.com-e5363e1140b7
Whiteboard: [autoland-in-queue]
http://hg.mozilla.org/integration/mozilla-inbound/rev/18208d0bb8d1
Flags: in-testsuite+
Keywords: checkin-needed
OS: Linux → All
Hardware: x86 → All
Target Milestone: --- → mozilla13
https://hg.mozilla.org/mozilla-central/rev/18208d0bb8d1
Status: ASSIGNED → RESOLVED
Closed: 14 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: