Closed Bug 1029695 Opened 6 years ago Closed 6 years ago

[rule view] Need null check for ruleview-propertyvalue query in getParentTextProperty

Categories

(DevTools :: Inspector, defect)

x86
macOS
defect
Not set
normal

Tracking

(Not tracked)

RESOLVED FIXED
Firefox 33

People

(Reporter: gl, Assigned: gl)

Details

Attachments

(3 files, 2 obsolete files)

Attached image Screenshot
STR:
1. Have a rule with no properties
2. Click to add properties
3. Move the mouse around the input box.
4. Observe console error for parent.querySelector(...) is null

rule-view#getParentTextProperty(node) needs a check to make sure .ruleview-propertyvalue exists or not and returns null as necessary.
Attached file test.html
Attached a test page
Assignee: nobody → gabriel.luong
Attached patch parentTextFix.patch (obsolete) — Splinter Review
Attachment #8445336 - Flags: review?(fayearthur)
Comment on attachment 8445336 [details] [diff] [review]
parentTextFix.patch

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

::: browser/devtools/styleinspector/rule-view.js
@@ +2860,5 @@
>   * @return {TextProperty}
>   */
>  function getParentTextProperty(node) {
>    let parent = getParentTextPropertyHolder(node);
> +  if (!parent || !parent.querySelector(".ruleview-propertyvalue")) {

instead of querySelectoring twice, cache it in a var, like:

var propValue = parent.querySelector(".ruleview-propertyvalue");
if (propValue) {
  return propValue.textProperty;
}
return null;
Attachment #8445336 - Flags: review?(fayearthur) → review+
Attached patch parentTextFix.patch (obsolete) — Splinter Review
https://tbpl.mozilla.org/?tree=Try&rev=07862b0133c0

I separated out the check for propValue since we want to make sure the parent isn't null first before doing querySelector.
Attachment #8445336 - Attachment is obsolete: true
Added r=harth to commit msg
Attachment #8446244 - Attachment is obsolete: true
Attachment #8446247 - Flags: review?(fayearthur)
Comment on attachment 8446247 [details] [diff] [review]
parentTextFix.patch

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

Looks good, thanks!
Attachment #8446247 - Flags: review?(fayearthur) → review+
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/328c548bb116
Status: NEW → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Whiteboard: [fixed-in-fx-team]
Target Milestone: --- → Firefox 33
Product: Firefox → DevTools
You need to log in before you can comment on or make changes to this bug.