Bug 78751

Summary: Add support for the translate attribute in html elements.
Product: WebKit Reporter: Pablo Flouret <pf>
Component: DOMAssignee: Nobody <webkit-unassigned>
Status: RESOLVED FIXED    
Severity: Normal CC: abarth, eric, mrakesh, rniwa, simon.fraser, webkit.review.bot
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch
none
Fixed changelogs and added assert none

Comment 1 Pablo Flouret 2012-02-15 15:25:09 PST
Created attachment 127249 [details]
Patch
Comment 2 Adam Barth 2012-02-15 15:43:20 PST
Comment on attachment 127249 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=127249&action=review

Is there any more to this attribute?  A link to the spec would be helpful so I can read up on it easily.  Some nits below.

> Source/WebCore/ChangeLog:8
> +        Add support for the translate attribute in html elements.
> +        https://bugs.webkit.org/show_bug.cgi?id=78751
> +
> +        Reviewed by NOBODY (OOPS!).
> +
> +        Test: fast/dom/HTMLElement/translate.html

Can you add some more information to the ChangeLog, like a link to the spec?

> Source/WebCore/html/HTMLElement.cpp:823
> +            if (mode != TranslateAttributeInherit)
> +                return mode == TranslateAttributeYes;

Should we ASSERT that the mode is either TranslateAttributeYes or TranslateAttributeNo in the body of this if?
Comment 3 Pablo Flouret 2012-02-15 15:50:09 PST
(In reply to comment #2)
> (From update of attachment 127249 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=127249&action=review
> 
> Is there any more to this attribute?  A link to the spec would be helpful so I can read up on it easily.  Some nits below.

The link to the spec is in the description field of this bug, to wit, http://www.whatwg.org/specs/web-apps/current-work/multipage/elements.html#attr-translate

I forgot to add it to the changelog, will do.

> 
> > Source/WebCore/html/HTMLElement.cpp:823
> > +            if (mode != TranslateAttributeInherit)
> > +                return mode == TranslateAttributeYes;
> 
> Should we ASSERT that the mode is either TranslateAttributeYes or TranslateAttributeNo in the body of this if?

We could.
Comment 4 Pablo Flouret 2012-02-15 15:58:23 PST
Created attachment 127260 [details]
Fixed changelogs and added assert
Comment 5 Adam Barth 2012-02-15 16:05:01 PST
Comment on attachment 127260 [details]
Fixed changelogs and added assert

Thanks!
Comment 6 WebKit Review Bot 2012-02-15 20:40:14 PST
Comment on attachment 127260 [details]
Fixed changelogs and added assert

Rejecting attachment 127260 [details] from commit-queue.

Failed to run "['/mnt/git/webkit-commit-queue/Tools/Scripts/webkit-patch', '--status-host=queues.webkit.org', '-..." exit_code: 2

Last 500 characters of output:
ools/ChangeLog
CONFLICT (content): Merge conflict in Tools/ChangeLog
Failed to merge in the changes.
Patch failed at 0001 [chromium] Rebaseline JPEG image results after r107389

When you have resolved this problem run "git rebase --continue".
If you would prefer to skip this patch, instead run "git rebase --skip".
To restore the original branch and stop rebasing run "git rebase --abort".

rebase refs/remotes/origin/master: command returned error: 1

Died at Tools/Scripts/update-webkit line 164.

Full output: http://queues.webkit.org/results/11527988
Comment 7 WebKit Review Bot 2012-02-15 21:42:23 PST
Comment on attachment 127260 [details]
Fixed changelogs and added assert

Clearing flags on attachment: 127260

Committed r107883: <http://trac.webkit.org/changeset/107883>
Comment 8 WebKit Review Bot 2012-02-15 21:42:28 PST
All reviewed patches have been landed.  Closing bug.
Comment 9 Simon Fraser (smfr) 2012-03-26 14:21:46 PDT
This broke mobile.orange.fr: bug 82246.
Comment 10 rakesh makluri 2012-04-11 21:29:55 PDT
Hi Adam,

  With this support our product is breaking now. Present we are loading and using the translate() API on HTML elements to translate. But now with this change it is breaking. Is there any way to override this attribute.

  We can change the API but, so many existing customers and our internal components are also using this.
  Please help with this.