WebKit Bugzilla
New
Browse
Log In
×
Sign in with GitHub
or
Remember my login
Create Account
·
Forgot Password
Forgotten password account recovery
RESOLVED FIXED
20226
BORDER attribute with the IMG tag, using percentage values not working.
https://bugs.webkit.org/show_bug.cgi?id=20226
Summary
BORDER attribute with the IMG tag, using percentage values not working.
deepak.sherveghar
Reported
2008-07-30 03:43:26 PDT
I have this simple HTML page. <HTML> <HEAD/> <BODY> <IMG SRC="abc.gif" border="0%" ALT="Alt Text"> </BODY> </HTML> Where width and height of "abc.gif" is 320x240. Expected result: A black border should surround the image, of a size that is the specified percentage of the available space, for <IMG> tag above. Actual result: No matter what the size of border specified in terms of percentage, we always see a SOLID border of width 3 pixels. Note that if we specify the border in terms of pixel's then everthing works fine. Also I have debugged with a webkit windows (cairo/CURL) build and found that the problem is with the cssParser. For boder=10% (anyvalue in terms of %) CSSParser::parseValue returns false. For border values specified in terms of pixels CSSParser::parseValue returns true and everything works fine.
Attachments
Patch
(6.93 KB, patch)
2011-05-10 05:51 PDT
,
Mihnea Ovidenie
no flags
Details
Formatted Diff
Diff
Same patch without tabs
(6.96 KB, patch)
2011-05-10 06:10 PDT
,
Mihnea Ovidenie
no flags
Details
Formatted Diff
Diff
Patch
(6.45 KB, patch)
2011-05-10 07:06 PDT
,
Mihnea Ovidenie
simon.fraser
: review-
Details
Formatted Diff
Diff
Simpler test case
(33.37 KB, application/zip)
2011-07-18 10:37 PDT
,
Mihnea Ovidenie
no flags
Details
Patch 2
(7.40 KB, patch)
2011-07-22 09:17 PDT
,
Mihnea Ovidenie
simon.fraser
: review+
simon.fraser
: commit-queue-
Details
Formatted Diff
Diff
Patch 3
(6.85 KB, patch)
2011-07-22 11:49 PDT
,
Mihnea Ovidenie
no flags
Details
Formatted Diff
Diff
Show Obsolete
(4)
View All
Add attachment
proposed patch, testcase, etc.
deepak.sherveghar
Comment 1
2008-07-30 03:48:07 PDT
(In reply to
comment #0
)
> I have this simple HTML page. > <HTML> > <HEAD/> > <BODY> > <IMG SRC="abc.gif" border="0%" ALT="Alt Text"> > </BODY> > </HTML> > > Where width and height of "abc.gif" is 320x240. > > Expected result: > A black border should surround the image, of a size that is the specified > percentage of the available space, for <IMG> tag above. > > Actual result: > No matter what the size of border specified in terms of percentage, we always > see a SOLID border of width 3 pixels. > > Note that if we specify the border in terms of pixel's then everthing works > fine. > > Also I have debugged with a webkit windows (cairo/CURL) build and found that > the problem is with the cssParser. > > For boder=10% (anyvalue in terms of %) CSSParser::parseValue returns false. > For border values specified in terms of pixels CSSParser::parseValue returns > true and everything works fine. >
sorry for small typo ..... the above IMG tag should be <IMG SRC="abc.gif" border="10%" ALT="Alt Text"> with border as 10% and not 0%.
Dave Hyatt
Comment 2
2008-07-30 11:30:35 PDT
What do other browsers do? I suspect they drop the % and treat the number like a pixel value.
deepak.sherveghar
Comment 3
2008-07-30 11:57:52 PDT
Other Desktop Browser's like IE 7 and firefox 2.0 do display the expected behaviour (border around the image). Also I installed FIREBUG (a tool that allows to inspect, edit and monitor CSS, HTML and JavaScript live in any web page). I inspected the HTML code and found that it does seem to remove the % sign. AS all i can see in the HTML code is <IMG SRC="abc.gif" border="10" ALT="Alt Text"> No percent sign. I Inspected and edited the border's value in terms of pixel and percent e.g border=10 and border=10% Both the above values for border (10 and 10%) show border with same thickness in firefox.
Dave Hyatt
Comment 4
2008-07-30 11:58:45 PDT
So we just need to strip % from the string we pass to the CSS parser. Sounds easy enough.
deepak.sherveghar
Comment 5
2008-07-30 12:40:19 PDT
I tried removing the '%' sign from the string .. it does render the border properly now. But Dave one question. I just couldnt figure out why does our css parser cant deal with '%' unit properly ??? It does seem to work with all other units like pcs and pixels ..
Mihnea Ovidenie
Comment 6
2011-05-10 05:51:44 PDT
Created
attachment 92941
[details]
Patch
WebKit Review Bot
Comment 7
2011-05-10 05:56:02 PDT
Attachment 92941
[details]
did not pass style-queue: Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'LayoutTests/ChangeLog', u'LayoutTests/fast..." exit_code: 1 Source/WebCore/ChangeLog:11: Line contains tab character. [whitespace/tab] [5] Total errors found: 1 in 9 files If any of these errors are false positives, please file a bug against check-webkit-style.
Mihnea Ovidenie
Comment 8
2011-05-10 06:10:18 PDT
Created
attachment 92944
[details]
Same patch without tabs Removed the tabs from the first patch.
Mihnea Ovidenie
Comment 9
2011-05-10 07:06:28 PDT
Created
attachment 92949
[details]
Patch Forgot to set up ? for review.
Simon Fraser (smfr)
Comment 10
2011-07-18 09:42:11 PDT
Comment on
attachment 92949
[details]
Patch Does this match other browsers, and the spec?
Simon Fraser (smfr)
Comment 11
2011-07-18 10:26:38 PDT
Normally invalid units cause the entire declaration to be dropped.
Mihnea Ovidenie
Comment 12
2011-07-18 10:37:03 PDT
Created
attachment 101171
[details]
Simpler test case Attached a simple test case containing a html and a png.
Tab Atkins
Comment 13
2011-07-18 10:41:36 PDT
The behavior of border="10%" should be to apply a 10px border. First, given that you are using a presentational attribute, we use HTML's rules for parsing it. The 'border' attribute is defined as "mapping to a pixel length property" <
http://www.whatwg.org/specs/web-apps/current-work/complete/rendering.html#maps-to-the-pixel-length-property
>, which means we first try to parse it with the "rules for parsing non-negative integers" <
http://www.whatwg.org/specs/web-apps/current-work/complete/common-microsyntaxes.html#rules-for-parsing-non-negative-integers
>. When you do that, it turns the value "10", which is then interpreted as the CSS length 10px. Note that if you were to use "border-width: 10%" in actual CSS, it would have no effect, because the border-width property is defined as taking "<length> | thin | medium | thick" in both CSS 2.1 and the current ED of the Backgrounds & Borders module. Thus, a declaration like that would be invalid and be ignored. Since this is an actual bug in our parser, I've marked this bug as confirmed.
Tab Atkins
Comment 14
2011-07-18 10:44:40 PDT
(In reply to
comment #8
)
> Created an attachment (id=92944) [details] > Same patch without tabs > > Removed the tabs from the first patch.
I'm not yet a reviewer, so I can't r- the patch, but I'd like to. While this solves the problem at hand, it would be better to implement the algorithm as the spec specifies, where you simply ignore whitespace and '+' on the front of the number, then grab as many contiguous digits as possible and interpret that as a length. Special-casing '%' here is incorrect.
Mihnea Ovidenie
Comment 15
2011-07-22 09:17:51 PDT
Created
attachment 101737
[details]
Patch 2 Second version of patch.
Tab Atkins
Comment 16
2011-07-22 10:34:12 PDT
(In reply to
comment #15
)
> Created an attachment (id=101737) [details] > Patch 2 > > Second version of patch.
I'm not yet experience enough to comment on the overall style, but I'm happy with the direction of this patch. Yay!
Simon Fraser (smfr)
Comment 17
2011-07-22 11:02:43 PDT
Comment on
attachment 101737
[details]
Patch 2 View in context:
https://bugs.webkit.org/attachment.cgi?id=101737&action=review
> Source/WebCore/html/HTMLImageElement.cpp:101 > +int HTMLImageElement::parseBorderWidthAttribute(Attribute* attr)
Does this need to be a class method? I don't see it using any members.
Mihnea Ovidenie
Comment 18
2011-07-22 11:49:39 PDT
Created
attachment 101748
[details]
Patch 3 The method to parse the border with attribute is made static as it does not touch any of the HTMLImageElement members. Also, i changed its return type to unsigned int.
WebKit Review Bot
Comment 19
2011-07-22 13:28:09 PDT
Comment on
attachment 101748
[details]
Patch 3 Clearing flags on attachment: 101748 Committed
r91601
: <
http://trac.webkit.org/changeset/91601
>
WebKit Review Bot
Comment 20
2011-07-22 13:28:14 PDT
All reviewed patches have been landed. Closing bug.
Mihnea Ovidenie
Comment 21
2011-09-02 01:07:40 PDT
***
Bug 48594
has been marked as a duplicate of this bug. ***
Note
You need to
log in
before you can comment on or make changes to this bug.
Top of Page
Format For Printing
XML
Clone This Bug