Closed Bug 180852 Opened 22 years ago Closed 22 years ago

[FIX]unknown namespace error reporting could be clearer

Categories

(Core :: CSS Parsing and Computation, defect)

defect
Not set
normal

Tracking

()

RESOLVED FIXED

People

(Reporter: timeless, Assigned: bzbarsky)

Details

(Keywords: modern)

Attachments

(1 file, 1 obsolete file)

CSS Error (chrome://messenger/skin/addressbook/addressbook.css :225.6): Unknown
namespace prefix '|'.  Selector expected Ruleset ignored due to bad selector.

<bz> the error reporting there is broken, though...
<bz> the bogus prefix is "html"
Attachment #106765 - Flags: review?(dbaron)
Keywords: modern
OS: Windows 2000 → All
Hardware: PC → All
Summary: addressbook triggers css style error → [FIX]unknown namespace error reporting could be clearer
Comment on attachment 106765 [details] [diff] [review]
Report the actual namespace causing the error.

> static void ReportUnexpectedToken(nsCSSScanner *sc,
>                                   nsCSSToken& tok,
>                                   const nsAString& err)
> {
>-  nsAutoString error(err + NS_LITERAL_STRING(" '"));
>+  // XXX Flatten the string so we don't append to a concatenation,
>+  // since that goes into an infinite loop.
>+  nsAutoString error(err);
>+  error += NS_LITERAL_STRING(" '");

No need for "XXX".  Cite bug 70083 instead.

>-        REPORT_UNEXPECTED_TOKEN(NS_LITERAL_STRING("Unknown namespace prefix"));
>+        REPORT_UNEXPECTED_TOKEN(
>+            NS_LITERAL_STRING("Unknown namespace prefix '") +
>+            buffer +
>+            NS_LITERAL_STRING("'; didn't expect")
>+          );

Should this be written REPORT_UNEXPECTED rather than REPORT_UNEXPECTED_TOKEN? 
What does buffer hold and what does mToken hold?

Same thing below.
Comment on attachment 106765 [details] [diff] [review]
Report the actual namespace causing the error.

Well, now that I read the bug I see what mToken holds.	So yeah, I think you
should just switch to REPORT_UNEXPECTED -- it's obvious enough if the error is
reported at the position of the |.
Attached patch Fix thoseSplinter Review
ah, right.  That was the bug#.
Attachment #106765 - Attachment is obsolete: true
Attachment #106767 - Flags: review?(dbaron)
Attachment #106765 - Flags: review?(dbaron)
fixed.
Status: NEW → RESOLVED
Closed: 22 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: