195318 - handlerelease uses wrong frame sometimes to resolve style

Status: NEW

(Core :: DOM: Selection, defect, P5)

Description

[mjudge]

we need to make sure that when a handlerelease call is made that just because a
container frame may get the event, that we pass the leaf frame to the call for
"AdjustingStyle" for selection.

Comment 1

[mjudge]

Attachment: patch for nsFrame.cpp

Comment 2

[mjudge]

end user view of this bug comes when you click below (in dead space) a line of
text that perhaps has a style that affects selection.  If that area is all ready
selected, and the user clicks below it, the code that sees the mouse up and
realizes that a drag did not take place fails to grab the proper frame for the
style information. This means that the user can click below selected text with
selection affecting style and still end up inside that text.  This is different
than if they clicked on the selection exactly.

Comment 3

[kinmoz]

Comment on attachment 115797 [details] [diff] [review]
patch for nsFrame.cpp

sr=[e-mail] with the following addressed:


==== Move this declaration under the |if| block closer to where it is actually
used. There's no need to declare it outside the |if| block if it's never used
outside the |if| block.



+      nsIFrame *resultFrame = this;
       if (NS_SUCCEEDED(result) && !mouseDown && me && me->clickCount < 2)
       {


==== You can lose the code that retrieves |shell| since |presShell| is already
available for use from earlier in the method.


+	 if (content != mContent) //get primary frame for style purposes
+	 {
+	   nsCOMPtr<nsIPresShell> shell;
+	   aPresContext->GetShell(getter_AddRefs(shell));
+	   if (shell)
+	   {
+	     if (NS_FAILED(shell->GetPrimaryFrameFor(content, &resultFrame)))
+	       resultFrame = this;
+	   }
+	 }



==== Can we lose the braces around this code and put a return in after the
variable declarations too?


       // do we have CSS that changes selection behaviour?
       {
	 PRBool    changeSelection;
	 nsCOMPtr<nsIContent>  selectContent;
	 PRInt32   newStart, newEnd;
-	 if (NS_SUCCEEDED(frameselection->AdjustOffsetsFromStyle(this,
&changeSelection, getter_AddRefs(selectContent), &newStart, &newEnd))
+	 if (NS_SUCCEEDED(frameselection->AdjustOffsetsFromStyle(resultFrame,
&changeSelection, getter_AddRefs(selectContent), &newStart, &newEnd))
	   && changeSelection)
	 {
	   content = selectContent;
	   startOffset = newStart;
	   endOffset = newEnd;
	 }
       }

Comment 4

[Jens Stutte [:jstutte]]

Bulk-downgrade of unassigned, >=5 years untouched DOM/Storage bugs' priority and severity.

If you have reason to believe this is wrong, please write a comment and ni :jstutte.