Note: There are a few cases of duplicates in user autocompletion which are being worked on.

'border-image' shorthand should not accept odd trailing slash

RESOLVED FIXED in mozilla14

Status

()

Core
CSS Parsing and Computation
P3
trivial
RESOLVED FIXED
5 years ago
5 years ago

People

(Reporter: kennyluck, Assigned: kennyluck)

Tracking

({dev-doc-complete})

unspecified
mozilla14
dev-doc-complete
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

(URL)

Attachments

(1 attachment, 3 obsolete attachments)

(Assignee)

Description

5 years ago
Gecko now accepts an odd trailing slash for the 'border-image-slice'/'border-image-width'/'border-image-width' part, following an older version of the CSS3 B&B spec. I raised that as an issue on www-style[1] and the editors decided on[2] Option C (disallow trailing slash), not my favorite option (which is D) but still falls into what I consider acceptable. I won't be very surprised if we end up having Option D in the final version of the spec though since that's what's implemented in WebKit.

However, I'll just submit a patch with mochitest implementing Option C in a short time. It shouldn't take more than an hour.

ps. This is relevant to bug 497995 but I don't have access to the bug metadata. May I get the access by the way?

[1] http://lists.w3.org/Archives/Public/www-style/2012Mar/0179
[2] http://lists.w3.org/Archives/Public/www-style/2012Mar/0605
(Assignee)

Comment 1

5 years ago
(In reply to Kang-Hao (Kenny) Lu from comment #0)
> ps. This is relevant to bug 497995 but I don't have access to the bug
> metadata. May I get the access by the way?

Hmm... It seems that I can do so as the reporter, but still want the access :p
(In reply to Kang-Hao (Kenny) Lu from comment #0)
> ps. This is relevant to bug 497995 but I don't have access to the bug
> metadata. May I get the access by the way?

I gave you canconfirm and editbugs.   See https://developer.mozilla.org/en/What_to_do_and_what_not_to_do_in_Bugzilla
Status: UNCONFIRMED → NEW
Ever confirmed: true
Component: Layout: Block and Inline → Style System (CSS)
QA Contact: layout.block-and-inline → style-system
(Assignee)

Comment 3

5 years ago
Created attachment 613837 [details] [diff] [review]
small patch with a Mochitest included
Assignee: nobody → kennyluck
Status: NEW → ASSIGNED
Attachment #613837 - Flags: review?(dbaron)
(Assignee)

Comment 4

5 years ago
(In reply to David Baron [:dbaron] from comment #2)
> (In reply to Kang-Hao (Kenny) Lu from comment #0)
> > ps. This is relevant to bug 497995 but I don't have access to the bug
> > metadata. May I get the access by the way?
> 
> I gave you canconfirm and editbugs.   See
> https://developer.mozilla.org/en/What_to_do_and_what_not_to_do_in_Bugzilla

Great! Thanks. Reading it now.
Comment on attachment 613837 [details] [diff] [review]
small patch with a Mochitest included

There's no need to add an entire new mochitest file to test this.

So instead of adding test_bug744243.html and modifying the Makefile.in, just add your two values to the invalid_values line for border-image in property_database.js, and check that test_property_syntax_errors.html shows failures without your patch.  (It might be worth adding a few other related tests, too.)

r=dbaron with that changed
Attachment #613837 - Flags: review?(dbaron) → review+
(Assignee)

Comment 6

5 years ago
Created attachment 613891 [details] [diff] [review]
comments addressed

(In reply to David Baron [:dbaron] from comment #5)
> Comment on attachment 613837 [details] [diff] [review]
> small patch with a Mochitest included
> 
> There's no need to add an entire new mochitest file to test this.
> 
> So instead of adding test_bug744243.html and modifying the Makefile.in, just
> add your two values to the invalid_values line for border-image in
> property_database.js, and check that test_property_syntax_errors.html shows
> failures without your patch.  (It might be worth adding a few other related
> tests, too.)

Done. I added two more tests (two trailing slashes, no number before slash).
Attachment #613837 - Attachment is obsolete: true
(Assignee)

Updated

5 years ago
Keywords: checkin-needed
https://hg.mozilla.org/integration/mozilla-inbound/rev/986091dac358

I made two changes:
 * tweaked the checkin comment to use the property name rather than the function it's parsed in
 * fixed a typo (intial -> initial)

Thanks for fixing this.
Priority: -- → P4
Target Milestone: --- → mozilla14
Priority: P4 → P3
Keywords: checkin-needed
(Assignee)

Comment 8

5 years ago
Created attachment 613949 [details] [diff] [review]
corrected a test and fixed a typo.

ms2ger mentioned that the patch is backed out in https://hg.mozilla.org/integration/mozilla-inbound/rev/46eacf64298a , and I noticed that "other_values" had an invalid value with trailing slash. I didn't realize that and didn't run the whole mochitest suite. Sorry :(

I also made a typo in the commit message "744234" should read "744243". These are now corrected.
Attachment #613891 - Attachment is obsolete: true
Maybe also worth listing that old value in the invalid_values line rather than dropping it entirely?
(Assignee)

Comment 10

5 years ago
Created attachment 614275 [details] [diff] [review]
add the deleted test to "invalid_values"

(In reply to David Baron [:dbaron] from comment #9)
> Maybe also worth listing that old value in the invalid_values line rather
> than dropping it entirely?

I reason I didn't add it ("url('border.png') 27 27 27 27 /") to "invalid_values" back that is because I wrote a test ("url('border.png') 1 /") independently which pretty much tests the same thing.

Anyway, I add "url('border.png') 27 27 27 27 /" to "invalid_values" now. I'm adding "checkin-needed" again.
Attachment #613949 - Attachment is obsolete: true
(Assignee)

Updated

5 years ago
Keywords: checkin-needed
http://hg.mozilla.org/integration/mozilla-inbound/rev/987f9a47cee9
Keywords: checkin-needed
(Assignee)

Updated

5 years ago
Status: ASSIGNED → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
https://hg.mozilla.org/mozilla-central/rev/987f9a47cee9
Keywords: dev-doc-needed
If I interpret the doc here correctly:

https://developer.mozilla.org/En/CSS/border-image

This looks like our doc is for the new behavior to me.

Mentioned on Firefox 14 for developers.
Keywords: dev-doc-needed → dev-doc-complete
You need to log in before you can comment on or make changes to this bug.