Open
Bug 684221
Opened 13 years ago
Updated 2 years ago
Test and fix HTMLTableCellElement attributes reflection
Categories
(Core :: DOM: Core & HTML, defect, P5)
Core
DOM: Core & HTML
Tracking
()
NEW
People
(Reporter: mounir, Unassigned)
References
Details
Attachments
(1 file, 1 obsolete file)
|
(deleted),
patch
|
smaug
:
review-
Ms2ger
:
feedback+
|
Details | Diff | Splinter Review |
Bug 673820 do not pass try because some tests are failing because of a bad implementation of the attributes reflection.
|
Reporter | |
Comment 1•13 years ago
|
||
Attachment #557828 -
Flags: review?(Olli.Pettay)
Attachment #557828 -
Flags: feedback?(Ms2ger)
|
|
Reporter | |
Comment 2•13 years ago
|
||
Status: NEW → ASSIGNED
Flags: in-testsuite+
Whiteboard: [needs review]
|
||
Comment 3•13 years ago
|
||
Comment on attachment 557828 [details] [diff] [review]
Patch v1
colspan and rowspan aren't actually supposed to be handled the same.
Attachment #557828 -
Flags: feedback?(Ms2ger) → feedback-
|
|
Reporter | |
Comment 4•13 years ago
|
||
Thanks Ms2ger, I misread the specs. This should fix it.
Attachment #557828 -
Attachment is obsolete: true
Attachment #557828 -
Flags: review?(Olli.Pettay)
Attachment #558193 -
Flags: review?(Olli.Pettay)
Attachment #558193 -
Flags: feedback?(Ms2ger)
|
|
||
Comment 5•13 years ago
|
||
Comment on attachment 558193 [details] [diff] [review]
Patch v1.1
>--- a/content/html/content/src/nsHTMLTableCellElement.cpp
>+++ b/content/html/content/src/nsHTMLTableCellElement.cpp
>+NS_IMPL_UINT_ATTR_NON_ZERO(nsHTMLTableCellElement, ColSpan, colspan)
>+NS_IMPL_UINT_ATTR_DEFAULT_VALUE(nsHTMLTableCellElement, RowSpan, rowspan, 1)
> if (aAttribute == nsGkAtoms::colspan) {
>+ return aResult.ParsePositiveIntValue(aValue);
> if (aAttribute == nsGkAtoms::rowspan) {
>+ return aResult.ParseNonNegativeIntValue(aValue);
Looks correct per spec, but you'll need to add the clamping to nsTableCellFrame::Get{Col,Row}Span.
Attachment #558193 -
Flags: feedback?(Ms2ger) → feedback+
|
||
Comment 6•13 years ago
|
||
Comment on attachment 558193 [details] [diff] [review]
Patch v1.1
> if (aAttribute == nsGkAtoms::colspan) {
>- PRBool res = aResult.ParseIntWithBounds(aValue, -1);
>- if (res) {
>- PRInt32 val = aResult.GetIntegerValue();
>- // reset large colspan values as IE and opera do
>- // quirks mode does not honor the special html 4 value of 0
>- if (val > MAX_COLSPAN || val < 0 ||
>- (0 == val && InNavQuirksMode(GetOwnerDoc()))) {
>- aResult.SetTo(1);
>- }
>- }
>- return res;
>+ return aResult.ParsePositiveIntValue(aValue);
Why is this ok?
> }
> if (aAttribute == nsGkAtoms::rowspan) {
>- PRBool res = aResult.ParseIntWithBounds(aValue, -1, MAX_ROWSPAN);
>- if (res) {
>- PRInt32 val = aResult.GetIntegerValue();
>- // quirks mode does not honor the special html 4 value of 0
>- if (val < 0 || (0 == val && InNavQuirksMode(GetOwnerDoc()))) {
>- aResult.SetTo(1);
>- }
>- }
>- return res;
>+ return aResult.ParseNonNegativeIntValue(aValue);
And this?
Can layout really handle huge values?
Doesn't this regress bug 141818, Bug 27993 and Bug 30332?
What does other browsers do?
Attachment #558193 -
Flags: review?(Olli.Pettay) → review-
|
|
Reporter | |
Comment 7•11 years ago
|
||
I will unlikely work on this any time soon so I am unassigning myself. Feel free to take it over.
Assignee: mounir → nobody
Whiteboard: [needs review]
|
||
Comment 8•6 years ago
|
||
https://bugzilla.mozilla.org/show_bug.cgi?id=1472046
Move all DOM bugs that haven’t been updated in more than 3 years and has no one currently assigned to P5.
If you have questions, please contact :mdaly.
Priority: -- → P5
|
||
Comment 10•6 years ago
|
||
No assignee, updating the status.
|
||
Updated•2 years ago
|
Severity: normal → S3
You need to log in
before you can comment on or make changes to this bug.
Description
•