add lldb summaries for nsIAtoms, nsTextNodes and nsTextFragments showing their text content

RESOLVED FIXED in mozilla29

Status

()

defect
RESOLVED FIXED
6 years ago
5 years ago

People

(Reporter: heycam, Assigned: heycam)

Tracking

Trunk
mozilla29
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [qa-])

Attachments

(1 attachment, 2 obsolete attachments)

Assignee

Description

6 years ago
Posted patch patch (obsolete) — Splinter Review
No description provided.
Attachment #8359563 - Flags: review?(ehsan)
Assignee

Comment 1

6 years ago
Posted patch patch v2 (obsolete) — Splinter Review
Make the string formatting function more lenient about the types it accepts, so we can print nsAStrings etc. later more easily.
Assignee: nobody → cam
Status: NEW → ASSIGNED
Attachment #8359580 - Flags: review?(ehsan)
Assignee

Updated

6 years ago
Attachment #8359563 - Attachment is obsolete: true
Attachment #8359563 - Flags: review?(ehsan)
Assignee

Comment 2

6 years ago
Posted patch patch v2.1Splinter Review
Sorry one more tweak.
Attachment #8359580 - Attachment is obsolete: true
Attachment #8359580 - Flags: review?(ehsan)
Attachment #8359584 - Flags: review?(ehsan)
Assignee

Updated

6 years ago
Blocks: 959452

Comment 3

6 years ago
Comment on attachment 8359584 [details] [diff] [review]
patch v2.1

Review of attachment 8359584 [details] [diff] [review]:
-----------------------------------------------------------------

This is pretty neat, I must say!

::: python/lldbutils/lldbutils/content.py
@@ +9,5 @@
> +        field = "m2b"
> +    else:
> +        field = "m1b"
> +    ptr = content_union.GetChildMemberWithName(field)
> +    return utils.format_string(ptr, min(length, 100))

Hmm, I don't think we should clamp the string length like this, please remove the clamping code.
Attachment #8359584 - Flags: review?(ehsan) → review+
https://hg.mozilla.org/mozilla-central/rev/bb7af8904cac
Status: ASSIGNED → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla29
Comment on attachment 8359584 [details] [diff] [review]
patch v2.1

>+    elif c == 0x07:
>+        return "\\a"
>+    elif c == 0x08:
>+        return "\\b"
>+    elif c == 0x0c:
>+        return "\\f"
>+    elif c == 0x0a:
>+        return "\\n"
>+    elif c == 0x0d:
>+        return "\\r"
>+    elif c == 0x09:
>+        return "\\t"
>+    elif c == 0x0b:
>+        return "\\v"
I would have thought numerical order would make more sense than alphabetical order of special character.

>+    elif c < 0x20 or c >= 0x80 and c <= 0xff:
>+        return "\\x%02x" % c
IIRC 0x7f is also unprintable.

>+    elif c >= 0x0100:
If you swap the conditions then you can write
elif c >= 0x0100:
    ...
elif c < 0x20 or c >= 0x7f:
    ...
else:
    etc.

>+    i = 0
>+    terminated = False
>+    while i < length:
>+        c = lldb_value.CreateValueFromAddress("x", ptr + i * size, char_type).GetValueAsUnsigned(0) & mask
>+        if c == 0:
>+            terminated = True
Strings are always the exact provided length; embedded nulls are completely legal, in fact you will only hit this condition in such a case. It's also unsafe to read the length character to check whether the string is terminated; you can look at the flags if you really want to be sure, but it's certainly wrong to add an ellipsis in any event.
Assignee

Comment 7

6 years ago
Neil, can you open a separate bug for these things?

Updated

5 years ago
Whiteboard: [qa-]
You need to log in before you can comment on or make changes to this bug.