Bug 22664
| Summary: | Rename and review uses of parseURL() | ||
|---|---|---|---|
| Product: | WebKit | Reporter: | David Kilzer (:ddkilzer) <ddkilzer> |
| Component: | WebCore Misc. | Assignee: | Nobody <webkit-unassigned> |
| Status: | RESOLVED CONFIGURATION CHANGED | ||
| Severity: | Normal | CC: | annevk, darin |
| Priority: | P2 | ||
| Version: | 528+ (Nightly build) | ||
| Hardware: | Mac | ||
| OS: | OS X 10.5 | ||
David Kilzer (:ddkilzer)
*SUMMARY
Bug 11850 Comment #4 quoting Darin Adler:
Here's what parseURL does:
1) Strip leading and trailing spaces.
2) Strip URL() if it surrounds the string, allowing any mix of uppercase and lowercase.
3) Strip leading and trailing spaces.
4) Strip balanced single or double quotes.
5) Strip leading and trailing spaces.
6) Strip all characters in the range U+0000-U+000C
It's very strange that we want to do this anywhere except in CSS. In fact, I
think it's a bug almost every where this function is used. Perhaps some of this
is needed but obviously (2) is something specific to CSS property values even
if some of the other behavior is useful.
Someone should create some tests cases and investigate the behavior of other
web browsers for the various places we call this function. And at the same time
we should probably rename it.
| Attachments | ||
|---|---|---|
| Add attachment proposed patch, testcase, etc. |
David Kilzer (:ddkilzer)
I think parseURL() should be renamed to parseCSSURL() (or parseURLForCSS()) and moved to be in KURL.h/KURL.cpp. (That gets rid of CSSHelper.cpp, leaving only CSSHelper.h with a few constants.)
Darin Adler
(In reply to comment #1)
> I think parseURL() should be renamed to parseCSSURL() (or parseURLForCSS()) and
> moved to be in KURL.h/KURL.cpp. (That gets rid of CSSHelper.cpp, leaving only
> CSSHelper.h with a few constants.)
I agree that parseCSSURL is a good name. But I don't think KURL.h is the header for this. It's not about URL handling, but rather about the CSS syntax for URL properties. I think the right header for this would be CSSParser.h.
Anne van Kesteren
This has been refactored.