Closed Bug 1340483 Opened 8 years ago Closed 8 years ago

Add a chrome-only API to preview the text to be auto-filled in an <input>

Categories

(Core :: Layout: Form Controls, defect)

Product:
Core
Component:
Layout: Form Controls
Type:
defect
Priority:
Not set
Severity:
normal

Tracking

()

Status:
RESOLVED FIXED
Milestone:
mozilla55
Tracking Flags:
Tracking Status
firefox55 --- fixed

People

(Reporter: MattN, Assigned: ralin)

References

(
URL
)

Details

(Whiteboard: [form autofill:M2])

User Story

Requirements
* chrome-only API to set and reset the previewed text
** e.g. input.mozPreviewValue = "foo@example.com"; // set the previewed text
** input.mozPreviewValue = ""; // Reset to the state of no previewed text
* While there is a non-empty value for the preview value, the preview text appears in anonymous content in the same position as .value or .placeholder text would. The .value text and .placeholder text doesn't appear.
* The previewed text doesn't affect the validity state of the input
* The preview text can be styled via a CSS pseudo element.
** I believe the spec currently only requires `color`.
* The anonymous content shouldn't be detectable by unprivileged content in any way e.g. drawImage or CSS selectors like :empty.
* No DOM events should fire when the API is called
* No mutation observers should be notified when the API is called
* .value doesn't change when the API is called

Attachments

(5 files)

Bug 1340483 - Part 1. Create empty anonymous node for autofill preview.
59 bytes, text/x-review-board-request
heycam
: review+
Details
Bug 1340483 - Part 2. Expose chrome-only previewValue attribute.
59 bytes, text/x-review-board-request
heycam
: review+
baku
: review+
Details
Bug 1340483 - Part 3. Enable preview function only when input is marked as autofill field.
59 bytes, text/x-review-board-request
heycam
: review+
Details
Bug 1340483 - Part 4. Update input visibility accordingly.
59 bytes, text/x-review-board-request
heycam
: review+
Details
Bug 1340483 - Part 5. Add basic layout style for preview node.
59 bytes, text/x-review-board-request
heycam
: review+
Details
When an autocomplete entry is selected/highlighted in an autocomplete dropdown with Form Autofill, all relevant fields that will be autofilled should preview what their values will be upon confirming the selection (e.g. clicking the left mouse button on the autocomplete row or hitting enter). For both privacy and compatibility, the previewed values shouldn't be exposed to the web page until the user confirms the selection. This means that no DOM events will fire on the <input> as the autocomplete selection changes causing previewed values to change. It also means that accessing .value on the input will return it's value from before previewing. This bug can just handle adding the anonymous content to render the text that would be filled. We will also probably need to be able to style the text using a pseudo-element (the UX spec has it at a lower opacity or lighter color?) so we may want to see if other browsers already have a name to re-use. Styling the background of the previewed field will be left to bug 740979. I imagine we can do something like @placeholder but the value set by this new API should have priority over @placeholder meaning @placeholder doesn't appear if we have preview text. We should be careful about privacy leaks of the previewed data e.g. not allowing the anonymous content to capture and drawn to a canvas untainted and ensure that CSS can't reveal the values either (e.g. using a technique like :visited). UX Spec: https://mozilla.invisionapp.com/share/TM8GRODVH#/screens/185446465
See Also: → 1340488
Assignee: nobody → ralin
Status: NEW → ASSIGNED
Comment on attachment 8848977 [details] Bug 1340483 - Part 1. Create empty anonymous node for autofill preview. Hey Cameron, I've made three patches based on last discussion we had in person in mozspace. (about creating a anon node for autofill preview inside <input>,<textarea>) Would you mind to have a quick look at these patches and give me feedback if you had some time? There are some missing parts that I'm still working on like: part 4 - handle visibility of input value and placeholder part 5 - add some tests. It would be great if you could give me some suggestions or directions to go. Thank you :-D
Attachment #8848977 - Flags: feedback?(cam)
Comment on attachment 8848977 [details] Bug 1340483 - Part 1. Create empty anonymous node for autofill preview. https://reviewboard.mozilla.org/r/121832/#review125704 ::: dom/html/nsTextEditorState.cpp:2298 (Diff revision 3) > + rv = mPreviewDiv->AppendChildTo(previewText, false); > + NS_ENSURE_SUCCESS(rv, rv); > + > + return NS_OK; Just like with the placeholder, we probably need to update the preview text at this point. ::: layout/forms/nsTextControlFrame.cpp:432 (Diff revision 3) > nsIContent* placeholder = txtCtrl->GetPlaceholderNode(); > if (placeholder && !(aFilter & nsIContent::eSkipPlaceholderContent)) > aElements.AppendElement(placeholder); > > + nsIContent* preview = txtCtrl->GetPreviewNode(); > + if (preview) > + aElements.AppendElement(preview); We need to consider whether we should skip the preview node when the nsIContent::eSkipPlaceholderContent flag is given. I think this is used by accessibility code. Better ask some a11y people.
Comment on attachment 8849473 [details] Bug 1340483 - Part 2. Expose chrome-only previewValue attribute. https://reviewboard.mozilla.org/r/122256/#review125708 ::: dom/html/nsTextEditorState.cpp:2707 (Diff revision 2) > + if (!mPreviewDiv) > + return; > + > + nsAutoString previewValue(aValue); > + > + nsContentUtils::RemoveNewlines(previewValue); Is this the right thing to do for textarea previews? ::: dom/webidl/HTMLInputElement.webidl:188 (Diff revision 2) > + [ChromeOnly, BinaryName="previewValue"] > + attribute DOMString mozPreviewValue; I don't think there's any particular need to use "mozPreviewValue" instead of just "previewValue", since it's not exposed to content anyway.
Comment on attachment 8848977 [details] Bug 1340483 - Part 1. Create empty anonymous node for autofill preview. I think this generally looks good. There might be some better code re-use we can do, as some of the placeholder / preview functions are similar. > part 4 - handle visibility of input value and placeholder For the placeholder, I think we can make UpdatePlaceholderVisibility take into account whether a preview is shown. And in the function that sets and clears the preview text, we'll need to call UpdatePlaceholderVisibility. For the input value, I'm not too sure what the right approach is. Maybe adding a class to the editor <div> (which is the mRootNode in nsTextEditorState, i.e. the first node appended to aElements in nsTextControlFrame::CreateAnonymousContent) that hides it? Just like these rules apply to the editor <div>: http://searchfox.org/mozilla-central/rev/c48398abd9f0f074c69f2223260939e30e8f99a8/layout/style/res/forms.css#192 (It's likely some of these rules that apply to placeholder you'll need to apply to the preview text as well.) I'm not sure if display:none or visibility:hidden would be more appropriate. I guess we don't want to set the actual text in the editor to hide it. Experiment and see what works. :-) Once the user selects the item from the autocomplete popup, we will actually set the text in the <input value> (or the <textarea> contents), right? And at that point we hide the preview?
Attachment #8848977 - Flags: feedback?(cam) → feedback+
Appreciate your feedback! That's really helpful. It took some time to digest and discuss with our team before I can give more context about your question, sorry. > Just like with the placeholder, we probably need to update the preview text at this point. I think we could omit it as we're sure that we won't have any content for preview node while creating node. > We need to consider whether we should skip the preview node when the nsIContent::eSkipPlaceholderContent flag is given. I think this is used by accessibility code. Better ask some a11y people. This is a topic worthy discussion :) I'll comment about this in another form-autofill a11y bug, however, I don't think we will put focus on accessibility unless it would lead a severe issue to user. > I don't think there's any particular need to use "mozPreviewValue" instead of just "previewValue", since it's not exposed to content anyway. agree! will update in next patch. > For the placeholder, I think we can make UpdatePlaceholderVisibility take into account whether a preview is shown. And in the function that sets and clears the preview text, we'll need to call UpdatePlaceholderVisibility. yeah, that would properly solve the visibility problem between preview and placeholder. > For the input value, I'm not too sure what the right approach is.... I think this is much complicated as there are different way/timing trigger visibility change of input value. But from the aspect of form autofill, we ensure that preview and input value won't exist at the same time. That said, we don't preview data for input that already has .value, so ideally there's no chance to display visually overlapped text unless someone misuse this API. I'm not sure if we need to guarantee the display correctness for all cases(including corner cased) in this stage. > Once the user selects the item from the autocomplete popup, we will actually set the text in the <input value> (or the <textarea> contents), right? Yes, once user selects, we populate profile by method: setUserInput(). >And at that point we hide the preview? Currently we have no plan to hide/show preview like placeholder does. My initial thought about the few timing when to operate previewValue: Set value: - popup opened and mouse hover/keyboard navigate to one of items Clear(all fields): - popup closed - user selects(click on) one of item - none of items selected ________ If we control properly, the display states would like below: [layers] - value SHOW EMPTY EMPTY EMPTY ... ... - placeholder HIDE HIDE SHOW EMPTY ... ... - preview EMPTY SHOW EMPTY SHOW ... ... *HIDE: has value but refrain from showing. Hope the my replies make sense to you, I will update the patches soon. Thanks :D
Blocks: 1300996
Hi Cameron, Could you help me to review these patch, if you don't mind? I'm afraid that I neither be familiar with DOM nor know how to write aesthetic cpp code, so haven't modified too much. > There might be some better code re-use we can do, as some of the placeholder / preview functions are similar. I spent some time on it, still have no solid idea about how to re-use those in an elegant way. There are some subtle distinctions between their call-site at different point in time. Thanks!
Can the autofill controller decide that the preview text for a given form field should be the empty string, and do we need to distinguish that case from an input element not showing a preview? This affects (1) whether the input element will be highlighted (in later bugs, I guess), and (2) whether a placeholder is shown, since in the current patches we will show the placeholder if the preview text is an empty string.
Flags: needinfo?(ralin)
(In reply to Cameron McCormack (:heycam) (away Apr 1-4) from comment #18) > Can the autofill controller decide that the preview text for a given form field should be the empty string Yes, we do something like below when preview happens: `input.previewValue = input.value ? "" : "preview text...";` and clear all fields' previewValue once preview end: `input.previewValue = ""` Front-end should be able to handle the visibility of preview and input text. A corner case would break this if someone change field's value during preview, that is the only point that would display input value and preview at the same time. However, I don't think user can break this as it's impossible for user to change previewed fields' value with popup opened. > (1) whether the input element will be highlighted Highlight(background color) was initially considered being done in DOM as well, but in the end we decided to change the style by EventState + CSS filter that controlled in front-end code. > (2) since in the current patches we will show the placeholder if the preview text is an empty string. I think it's desirable for us. Thanks :)
Flags: needinfo?(ralin)
(In reply to Ray Lin[:ralin] from comment #19) > Front-end should be able to handle the visibility of preview and input text. > A corner case would break this if someone change field's value during > preview, that is the only point that would display input value and preview > at the same time. However, I don't think user can break this as it's > impossible for user to change previewed fields' value with popup opened. That can happen if there is script running on the page, e.g. doing setTimeout(function() { inputElement.value = "something"; }, 1000); and then the user starts interacting with the autofill UI. > > (2) since in the current patches we will show the placeholder if the preview text is an empty string. > I think it's desirable for us. OK.
(In reply to Cameron McCormack (:heycam) (away Apr 1-4) from comment #20) > That can happen if there is script running on the page, e.g. doing > > setTimeout(function() { inputElement.value = "something"; }, 1000); > > and then the user starts interacting with the autofill UI. Yeah, that's what I'm worry about, visibility would be broken if this case. Would adding a method like: UpdatePreviewVisibility or checking value.IsEmpty in BuildDisplayList help?
(In reply to Ray Lin[:ralin] from comment #21) > Yeah, that's what I'm worry about, visibility would be broken if this case. > Would adding a method like: UpdatePreviewVisibility or checking > value.IsEmpty in BuildDisplayList help? Yes, I think adding a method UpdatePreviewVisibility makes sense. Or maybe it makes sense to have a single UpdateVisibility function that can update both placeholder and preview text visibility? Not sure.
I see. I think single UpdateVisibility is simpler as we don't need to care about the callsites for UpdatePreviewVisibility, though, on the other hand, I'm worry about mixing standard and non-standard feature into one place. I'll try UpdateVisibility first, and then see if it looks fine.
It's an implementation detail, and not exposed to content or anything, so I think it's fine to mix those two.
Part 4 patch updated according to your comment. I've had a quick test on visibility including the case you mentioned in comment 20, and the result seems good. Thanks.
Comment on attachment 8849473 [details] Bug 1340483 - Part 2. Expose chrome-only previewValue attribute. https://reviewboard.mozilla.org/r/122256/#review125708 > Is this the right thing to do for textarea previews? After discussion, we think it's fine to remove those format specifier for now, since we don't want the chance to show multiline in input more to show single line in <textarea>.
Comment on attachment 8848977 [details] Bug 1340483 - Part 1. Create empty anonymous node for autofill preview. https://reviewboard.mozilla.org/r/121832/#review130212 This looks good, but I'd like to see it again after the refactoring. ::: dom/html/nsTextEditorState.cpp:2312 (Diff revision 5) > +nsresult > +nsTextEditorState::CreatePreviewNode() > +{ Since CreatePlaceholderNode and CreatePreviewNode are very similar, let's factor out the common functionality into a separate method, which creates a <div> and its child text node and returns it. The callers of this separate method can then be in charge of setting the class="" (for preview), and the existing assertion and call to UpdatePlaceholderText (for placeholder). ::: dom/html/nsTextEditorState.cpp:2340 (Diff revision 5) > + nsAutoString classValue; > + classValue.AppendLiteral("preview-div"); > + > + rv = mPreviewDiv->SetAttr(kNameSpaceID_None, nsGkAtoms::_class, > + classValue, false); For this you can do: mPreviewDiv->SetAttr(..., NS_LITERAL_STRING("preview-div"), ...); This will avoid copying the "preview-div" literal string into the buffer of nsAutoString. https://developer.mozilla.org/en-US/docs/Mozilla/Tech/XPCOM/Guide/Internal_strings#Literal_Strings ::: layout/forms/nsTextControlFrame.cpp:437 (Diff revision 5) > + if (preview) > + aElements.AppendElement(preview); Nit: put braces around this.
Attachment #8848977 - Flags: review?(cam) → review-
Comment on attachment 8849473 [details] Bug 1340483 - Part 2. Expose chrome-only previewValue attribute. https://reviewboard.mozilla.org/r/122256/#review129298 You'll need to get a DOM peer's review on the .webidl file changes, too. ::: dom/html/nsTextEditorState.cpp:2750 (Diff revision 4) > + NS_ASSERTION(mPreviewDiv, "This function should not be called if " > + "mPreviewDiv isn't set"); Nit: align the string on the second line. (Same in the other method.) ::: dom/webidl/HTMLTextAreaElement.webidl:85 (Diff revision 4) > + > + [ChromeOnly] > + attribute DOMString previewValue; This doesn't seem to fall in the category of "Mozilla extensions also defined on nsIDOMHTMLTextAreaElement". And it's also not in the other |partial interface|'s category of things mirrored in nsIDOMNSEditableElement. So I suggest making a new |partial interface| definition to add previewValue.
Attachment #8849473 - Flags: review?(cam) → review+
Comment on attachment 8849920 [details] Bug 1340483 - Part 3. Enable preview function only when input is marked as autofill field. https://reviewboard.mozilla.org/r/122668/#review130216 ::: dom/html/HTMLInputElement.h:1640 (Diff revision 3) > bool mIsDraggingRange : 1; > bool mNumberControlSpinnerIsSpinning : 1; > bool mNumberControlSpinnerSpinsUp : 1; > bool mPickerRunning : 1; > bool mSelectionCached : 1; > + bool mIsPreviewEnabled: 1; Nit: space before the ":". ::: dom/html/HTMLInputElement.cpp:2949 (Diff revision 3) > + mIsPreviewEnabled = true; > + nsLayoutUtils::PostRestyleEvent(this, nsRestyleHint(0), nsChangeHint_ReconstructFrame); Can you add a comment in here explaining why we need to reconstruct the frame for the <input> element? ::: dom/html/HTMLTextAreaElement.cpp:356 (Diff revision 3) > + mIsPreviewEnabled = true; > + nsLayoutUtils::PostRestyleEvent(this, nsRestyleHint(0), nsChangeHint_ReconstructFrame); (And copy the comment here too.) ::: layout/forms/nsTextControlFrame.h:342 (Diff revision 3) > // these packed bools could instead use the high order bits on mState, saving 4 bytes > bool mEditorHasBeenInitialized; > bool mIsProcessing; > // Keep track if we have asked a placeholder node creation. > bool mUsePlaceholder; > + bool mUsePreview; Add a comment above this, something like "Similarly for preview node creation.".
Attachment #8849920 - Flags: review?(cam) → review+
Comment on attachment 8853280 [details] Bug 1340483 - Part 4. Update input visibility accordingly. https://reviewboard.mozilla.org/r/125358/#review130220 r=me with these comments addressed. ::: dom/html/HTMLInputElement.h:239 (Diff revision 2) > NS_IMETHOD_(Element*) GetRootEditorNode() override; > NS_IMETHOD_(Element*) CreatePlaceholderNode() override; > NS_IMETHOD_(Element*) GetPlaceholderNode() override; > NS_IMETHOD_(Element*) CreatePreviewNode() override; > NS_IMETHOD_(Element*) GetPreviewNode() override; > - NS_IMETHOD_(void) UpdatePlaceholderVisibility(bool aNotify) override; > + NS_IMETHOD_(void) UpdateVisibility(bool aNotify) override; Since this method is on HTMLInputElement, "UpdateVisibility" sounds like it is updating the visibility of the whole <input> element. Is there a name you can use that means "placeholder and preview text"? Maybe "UpdateOverlayTextVisibility" or something? ::: dom/html/HTMLTextAreaElement.h:108 (Diff revision 2) > NS_IMETHOD_(Element*) GetRootEditorNode() override; > NS_IMETHOD_(Element*) CreatePlaceholderNode() override; > NS_IMETHOD_(Element*) GetPlaceholderNode() override; > NS_IMETHOD_(Element*) CreatePreviewNode() override; > NS_IMETHOD_(Element*) GetPreviewNode() override; > - NS_IMETHOD_(void) UpdatePlaceholderVisibility(bool aNotify) override; > + NS_IMETHOD_(void) UpdateVisibility(bool aNotify) override; Same here. ::: dom/html/nsITextControlElement.h:195 (Diff revision 2) > NS_IMETHOD_(void) InitializeKeyboardEventListeners() = 0; > > /** > * Update the placeholder visibility based on the element's state. > */ > - NS_IMETHOD_(void) UpdatePlaceholderVisibility(bool aNotify) = 0; > + NS_IMETHOD_(void) UpdateVisibility(bool aNotify) = 0; And here. Also, please update the comment to say that it updates the visibility of both the placeholder and preview text. ::: dom/html/nsITextControlElement.h:205 (Diff revision 2) > NS_IMETHOD_(bool) GetPlaceholderVisibility() = 0; > > /** > + * Returns the current expected preview visibility state. > + */ > + Nit: move this blank line down one line. ::: layout/forms/nsTextControlFrame.cpp:1234 (Diff revision 2) > // Update the display of the placeholder value if needed. > // We don't need to do this if we're about to initialize the > // editor, since EnsureEditorInitialized takes care of this. > if (mUsePlaceholder && !aBeforeEditorInit) > { > AutoWeakFrame weakFrame(this); > - txtCtrl->UpdatePlaceholderVisibility(aNotify); > + txtCtrl->UpdateVisibility(aNotify); > NS_ENSURE_STATE(weakFrame.IsAlive()); > } I think we need to do this if mUsePreview is true, too. So, check that in the condition as well, and update the comment. ::: layout/forms/nsTextControlFrame.cpp:1358 (Diff revision 2) > - if (kid->GetContent() != txtCtrl->GetPlaceholderNode() || > - txtCtrl->GetPlaceholderVisibility()) { > + if ((kid->GetContent() != txtCtrl->GetPlaceholderNode() || > + txtCtrl->GetPlaceholderVisibility()) && > + (kid->GetContent() != txtCtrl->GetPreviewNode() || > + txtCtrl->GetPreviewVisibility())) { I find this condition a bit hard to read, this way around. :-) What do you think about this: if (!((kid->GetContent() == txtCtrl->GetPlaceholderNode() && !txtCtrl->GetPlaceholderVisibility()) || (kid->GetContent() == txtCtrl->GetPreviewNode() && !txtCtrl->GetPreviewVisibility()))) { ... }
Attachment #8853280 - Flags: review?(cam) → review+
Comment on attachment 8853281 [details] Bug 1340483 - Part 5. Add basic layout style for preview node. https://reviewboard.mozilla.org/r/125360/#review130230 ::: layout/style/res/forms.css:221 (Diff revision 2) > * The placeholder should be ignored by pointer otherwise, we might have some > * unexpected behavior like the resize handle not being selectable. Update this comment to say "placeholder or preview". ::: layout/style/res/forms.css:226 (Diff revision 2) > opacity: 0.54; > } > > -textarea::placeholder { > +input > .preview-div, > +textarea > .preview-div { > + opacity: unset; > +} Instead of this, can you remove the |opacity: 0.54| from the rule that applies to placeholders and previews, and add a separate rule that only applies to placeholders?
Attachment #8853281 - Flags: review?(cam) → review+
Thank you so much Cameron, I've fixed all the issues, could you check if the refactoring in part 1 make sense to you?
Hi baku, Nice to meet you over bugzilla. Could you help me review part 2 as I made some changes to .webidl? Thanks
Comment on attachment 8848977 [details] Bug 1340483 - Part 1. Create empty anonymous node for autofill preview. https://reviewboard.mozilla.org/r/121832/#review130898 The refactoring looks great, thanks! ::: dom/html/nsTextEditorState.cpp:2264 (Diff revision 6) > +nsTextEditorState::CreateEmptyDivNode() > +{ > + MOZ_ASSERT(mBoundFrame); > + > + nsIPresShell *shell = mBoundFrame->PresContext()->GetPresShell(); > + MOZ_ASSERT(shell); (I'm not super sure about how safe it is just to assert these things, instead of the NS_ENSURE_TRUE calls that we currently do, but it's probably fine.)
Attachment #8848977 - Flags: review?(cam) → review+
(In reply to Cameron McCormack (:heycam) from comment #44) > (I'm not super sure about how safe it is just to assert these things, > instead of the NS_ENSURE_TRUE calls that we currently do, but it's probably > fine.) Thanks! I'm not sure about that either. IIRC, it seems not able to put NS_ENSURE_TRUE inside the method that returning something other than nsresult, so I guess probably MOZ_ASSERT can do something similar here, though I know very little about the safety problem of assertion :-)
Hi Cameron, I've just pushed a try this morning[0], but got a bunch of failure about assertion number. Is it possible that those are broken by my patches? Since some of the failed tests even have no <input>, <textarea> inside, I could not sure how that is related. However, I don't see other try cause such amount of failure around these two days, so I'm a bit confused. (Or perhaps, I rebased on a bad commit?) Thanks :) [0] Latest try: https://treeherder.mozilla.org/#/jobs?repo=try&revision=bf6889ccb0405b135f6fd59f4d42ec3ca583e243 * please omit commit "add mock data to result"
Flags: needinfo?(cam)
I found the assertion failure around: https://treeherder.mozilla.org/logviewer.html#?job_id=90330262&repo=try&lineNumber=14464-14472 So, perhaps, we could simply return an empty string if mPreviewDiv has been initialized yet, instead of adding an assertion at beginning of GetPreviewText().
Whiteboard: [form autofill] → [form autofill:M2]
The latest try looks fine after I removed the assertion: https://treeherder.mozilla.org/#/jobs?repo=try&revision=70df0bc589b618987314d6095c5c95baca818606&selectedJob=90890802 --------------- I'm sorry to bother you, Andrea. In addition to the review+ from Cameron, we would need a extra review from DOM peer to ensure the correctness of the changes about .webidl. If you get some time, could you kindly help to review part 2 patch? Thanks for your help.
Flags: needinfo?(cam) → needinfo?(amarchesini)
Comment on attachment 8849473 [details] Bug 1340483 - Part 2. Expose chrome-only previewValue attribute. https://reviewboard.mozilla.org/r/122256/#review133720 ::: dom/html/nsTextEditorState.cpp:2728 (Diff revision 5) > } > > void > +nsTextEditorState::SetPreviewText(const nsAString& aValue, bool aNotify) > +{ > + NS_ASSERTION(mPreviewDiv, "This function should not be called if " is this possible or not? I prefer to have a MOZ_ASSERT here. ::: dom/html/nsTextEditorState.cpp:2738 (Diff revision 5) > + return; > + > + nsAutoString previewValue(aValue); > + > + nsContentUtils::RemoveNewlines(previewValue); > + NS_ASSERTION(mPreviewDiv->GetFirstChild(), "preview div has no child"); I prefer to have MOZ_ASSERT instead. ::: dom/html/nsTextEditorState.cpp:2752 (Diff revision 5) > + > + // If we don't have a preview div, there's nothing to do. > + if (!mPreviewDiv) > + return; > + > + NS_ASSERTION(mPreviewDiv->GetFirstChild(), "preview div has no child"); same here.
Attachment #8849473 - Flags: review?(amarchesini) → review+
Flags: needinfo?(amarchesini)
> is this possible or not? I prefer to have a MOZ_ASSERT here Yes, it's possible. I've changed those to MOZ_ASSERT, thank you :baku :)
Keywords: checkin-needed
Pushed by ryanvm@gmail.com: https://hg.mozilla.org/integration/autoland/rev/cfdcc2aa9513 Part 1. Create empty anonymous node for autofill preview. r=heycam https://hg.mozilla.org/integration/autoland/rev/c3dfc1597192 Part 2. Expose chrome-only previewValue attribute. r=baku,heycam https://hg.mozilla.org/integration/autoland/rev/02c944a0dca4 Part 3. Enable preview function only when input is marked as autofill field. r=heycam https://hg.mozilla.org/integration/autoland/rev/6da00d83aa3c Part 4. Update input visibility accordingly. r=heycam https://hg.mozilla.org/integration/autoland/rev/59398ba02c9d Part 5. Add basic layout style for preview node. r=heycam
Keywords: checkin-needed
See Also: → 1429684
Depends on: 1556501
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: